Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dockerfile: put some XDG dirs under /tmp #16304

Merged
merged 1 commit into from
Jul 19, 2024

Conversation

woodruffw
Copy link
Member

Opening this up for consideration/feedback, not sure if this is the best approach yet 🙂

Background context:

  1. PEP 740 requires us to maintain a TUF repository containing the Sigstore root of trust (essentially a bundle of public keys/key materials for verifying the attestations uploaded to PyPI)
  2. This TUF repository needs to get stored somewhere on disk; sigstore-python currently uses platformdirs to place it somewhere sensible under the user's local data/cache dirs
  3. This doesn't work on Warehouse deployments, since Warehouse seems to run as uid=nobody with $HOME=/nonexistent, meaning that the XDG dirs don't exist.
  4. This gracefully addresses hacks around the above by creating two new directories under /tmp and using them as Warehouse's XDG dirs.

From a functionality/correctness perspective, this should have no impact on Warehouse: nothing else appears to use the XDG dirs and, if anything in the future does, it should be redirected to these new ones.

From a security perspective: this puts the XDG dirs in a "global" temporary directory within the container. However (AFAICT), Warehouse intentionally runs as nobody with a rootless configuration to mitigate this kind of minor risk.

CC @di

@woodruffw woodruffw requested a review from a team as a code owner July 18, 2024 21:02
@woodruffw woodruffw self-assigned this Jul 18, 2024
@woodruffw woodruffw mentioned this pull request Jul 18, 2024
25 tasks
Copy link
Member

@di di left a comment

Choose a reason for hiding this comment

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

This seems fine to me but will let @ewdurbin and/or @miketheman weigh in as well.

@ewdurbin
Copy link
Member

that all checks out.

would it make sense to have cabotage do this in its wrapper layer, since that is where the user clobbering happens? also then other stuff running there benefits.

if this goes well, adding to https://github.com/cabotage/cabotage-app/blob/e50890c43353e6a21361d8abf1fd4f8e0d7e46e8/cabotage/utils/release_build_context.py#L3-L11 seems like a good move.

@ewdurbin ewdurbin merged commit 6c1ffd1 into pypi:main Jul 19, 2024
17 checks passed
@ewdurbin ewdurbin deleted the ww/xdg-in-tmp branch July 19, 2024 12:24
@woodruffw
Copy link
Member Author

would it make sense to have cabotage do this in its wrapper layer, since that is where the user clobbering happens? also then other stuff running there benefits.

I think so, although I'm curious about testing this -- I think it'd be good to have a "backstop" type test to ensure these paths are functional, and I'm not sure if that's easier to do in Cabotage or in Warehouse 😅

@woodruffw
Copy link
Member Author

Yeah, looks like this needs to happen in Cabotage -- the nobody user still won't have R/W access to these dirs as-is. I'll tweak the backstop test and send a PR up there.

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.

3 participants