Skip to content

Secure server_data_dir creation with nested temp dir#173

Merged
minrk merged 2 commits intojupyterhub:mainfrom
dometto:patch-2
Feb 15, 2026
Merged

Secure server_data_dir creation with nested temp dir#173
minrk merged 2 commits intojupyterhub:mainfrom
dometto:patch-2

Conversation

@dometto
Copy link
Copy Markdown
Contributor

@dometto dometto commented Nov 24, 2025

Resolves #172

Wrap server_data_dir in another temporary directory to maintain security.

Resolves jupyterhub#172

Wrap server_data_dir in another temporary directory to maintain security.
@dometto
Copy link
Copy Markdown
Contributor Author

dometto commented Nov 24, 2025

I'm a bit puzzled as to why rserver changes the permissions on server-data-dir. There may be a configuration option somewhere to tweak this behavior, but on the other hand this is a simple enough fix.

# we create the server_data_dir inside another temp dir,
# as rserver seems to insists on changing its permissions to 777.
# wrapping it in the first tempdir insists the contents of server_data_dir stay secure.
server_data_dir = tempfile.mkdtemp(dir=tempfile.mkdtemp())
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we want to explicitly make the outer dir private, then, if this is the reason? As it is, umask will usually set the default permissions, which are often world-readable by default (umask 022).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Happy to change, but according to the mkdtemp docs, the directory is readable, writable, and searchable only by the creating user ID.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ping @minrk

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for clarifying!

@dometto
Copy link
Copy Markdown
Contributor Author

dometto commented Feb 2, 2026

Still waiting for feedback on this PR. Anything I can do to help this get merged?

@minrk minrk merged commit 341bfa9 into jupyterhub:main Feb 15, 2026
1 check passed
@dometto
Copy link
Copy Markdown
Contributor Author

dometto commented Feb 23, 2026

Thanks for merging this! ❤️ Would it be possible to do a new release that includes these changes?

@dometto
Copy link
Copy Markdown
Contributor Author

dometto commented Mar 25, 2026

@minrk I'm thinking that since this addresses a slight security concern, that probably merits a new pypi release of this package (see #175). In our deployments we'd love to be able to depend on the package instead of a specific github commit. Hope this is not too much trouble!

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

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Server-data-dir permissions

3 participants