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

Revert "Revert "Update to Elixir 1.18 (#132)" (#133)" #134

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

jiegillet
Copy link
Contributor

Some time after pushing the 1.18 update, several students had the following issue

12:47:06.474 [error] GenServer Mix.Sync.PubSub terminating
** (File.Error) could not make directory (with -p) "/tmp/mix_pubsub/P2SrvbEiUDXbwyyvCwHtmA": permission denied
    (elixir 1.18.1) lib/file.ex:346: File.mkdir_p!/1
    (mix 1.18.1) lib/mix/sync/pubsub.ex:222: Mix.Sync.PubSub.create_subscription_file/2
    (mix 1.18.1) lib/mix/sync/pubsub.ex:142: Mix.Sync.PubSub.handle_call/3
    (stdlib 6.2) gen_server.erl:2381: :gen_server.try_handle_call/4
    (stdlib 6.2) gen_server.erl:2410: :gen_server.handle_msg/6
    (stdlib 6.2) proc_lib.erl:329: :proc_lib.init_p_do_apply/3
Last message (from Mix.PubSub.Subscriber): {:subscribe, #PID<0.111.0>, "/tmp/solution_i2Nb99z1Gd/_build/test"}
State: %{port: nil, hash_to_pids: %{}}
Client Mix.PubSub.Subscriber is alive

    (stdlib 6.2) gen.erl:241: :gen.do_call/4
    (elixir 1.18.1) lib/gen_server.ex:1125: GenServer.call/3
    (mix 1.18.1) lib/mix/sync/pubsub.ex:44: Mix.Sync.PubSub.subscribe/1
    (mix 1.18.1) lib/mix/pubsub/subscriber.ex:21: Mix.PubSub.Subscriber.init/1
    (stdlib 6.2) gen_server.erl:2229: :gen_server.init_it/2
    (stdlib 6.2) gen_server.erl:2184: :gen_server.init_it/6
    (stdlib 6.2) proc_lib.erl:329: :proc_lib.init_p_do_apply/3

12:47:06.574 [notice] Application mix exited: shutdown
** (ArgumentError) errors were found at the given arguments:

  * 1st argument: the table identifier does not refer to an existing ETS table

    (stdlib 6.2) :ets.lookup(Mix.State, :debug)
    (mix 1.18.1) lib/mix/state.ex:26: Mix.State.get/2
    (mix 1.18.1) lib/mix/cli.ex:112: Mix.CLI.run_task/2

I believe that this is because when we build the project using mix, the /tmp/mix_pubsub folder is created. But since the build is created by root, appuser will later be unable to modify it.
I modified the build to clear /tmp before passing over to appuser and it worked locally for me.

What this PR is missing is a good explanation of why this wasn't detected before and a new test to prevent similar issues.
When I used bin/run-in-docker.sh slug /path/to/exercise ., it all worked fine. I could only reproduce the issue by using the docker container interactively with docker run -it --entrypoint /bin/bash exercism/elixir-test-runner and running bin/run.sh slug /opt/test-runner/test/hello-world/ /tmp.
Suggestions welcome.

@jiegillet
Copy link
Contributor Author

Also, is this worth signalling to the Elixir core team? Maybe they could fix this by setting the permissions of this new directory to any user, or is it a security risk?

Copy link
Contributor

@neenjaw neenjaw left a comment

Choose a reason for hiding this comment

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

Assuming that primary problem is that the tmp/* folder is owned by root, this is not an elixir core issue. This is because docker containers run as root inside the image unless told to run as another using the USER directive.

Dockerfile Outdated
Comment on lines 26 to 27
# clear temp files created by root to avoid permission issues
RUN rm -rf /tmp/*
Copy link
Contributor

@neenjaw neenjaw Jan 10, 2025

Choose a reason for hiding this comment

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

This may work, but the typical way to separate between compilation stages and running stages of a docker container is using a multi-stage build, which would look like this:

# Stage 1: Build
FROM hexpm/elixir:1.18.1-erlang-27.2-debian-bookworm-20241223 as builder

# Get the source code
WORKDIR /opt/test-runner
COPY . .

# Compile the formatter
WORKDIR /opt/test-runner/exercism_test_helper
RUN mix local.rebar --force && \
  mix local.hex --force && \
  mix deps.get && \
  MIX_ENV=test mix compile

# Build the escript
RUN MIX_ENV=prod mix escript.build

# Stage 2: Runner
FROM hexpm/elixir:1.18.1-erlang-27.2-debian-bookworm-20241223

# Install SSL ca certificates and dependencies
RUN apt-get update && \
  apt-get install bash jo jq -y

# Create appuser
RUN useradd -ms /bin/bash appuser

# Set working directory
WORKDIR /opt/test-runner

# Copy the compiled application from the builder stage
COPY --from=builder /opt/test-runner .

USER appuser

# Entrypoint script
ENTRYPOINT ["/opt/test-runner/bin/run.sh"]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually tried that, copying from the analyzer dockerfile, but I didn't manage to make it work, I'll try harder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jiegillet
Copy link
Contributor Author

Assuming that primary problem is that the tmp/* folder is owned by root, this is not an elixir core issue. This is because docker containers run as root inside the image unless told to run as another using the USER directive.

I'd say it's borderline. The idea of this Mix.Sync.PubSub thing is to broadcast when there's a compilation change, so that other processes can listen in and act accordingly and /tmp/mix_pubsub is at the core of it. But setting it up that way means that only one user at a time can run Elixir, and it's a little bit limiting. I'd say it might still be worth considering on the Elixir side.

@angelikatyborska
Copy link
Member

But setting it up that way means that only one user at a time can run Elixir, and it's a little bit limiting. I'd say it might still be worth considering on the Elixir side.

Can we come up with a reasonable demo scenario where this problem would be shown? I was trying to come up with something but...

  • I think the mix_pubsub directory is only created when running mix tasks, please correct me if I'm wrong. So I set up an 'echo' mix task (copy pasted an example from here https://hexdocs.pm/mix/1.12/Mix.Task.html)
  • When I run the task as my user, tmp directories mix_pubsub and mix_lock appear (note that on macos it's not in /tmp)
  • When I run the task as root, sudo mix echo foo, those directories are not created.

So I was unable to create a scenario where, with normal usage, mix_pubsub would be created as belonging to root, and then prevent a non-root user from running a mix task.

Maybe this can be better recreated in Docker? Or maybe with two non-root users?

Setting up a second non-root user on my own machine would be a pain in the ass. But I used to work like that, with two users. When I used one laptop for work and for private stuff. So it is a possible scenario. If that will cause issues for users, it's worth reproducing and reporting to the Elixir team...

@angelikatyborska
Copy link
Member

PS: I could merge this PR after work today, in ~9h, so that I have time to react if it goes wrong again.

@jiegillet
Copy link
Contributor Author

jiegillet commented Jan 10, 2025

I could reproduce it with two normal users trying to compile normal projects, and I think it's worth opening an issue, this could happen for any project created by different users.

➜  ~ whoami
jie_old
➜  ~ elixir -v                             
Erlang/OTP 27 [erts-15.2] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit] [dtrace]

Elixir 1.18.1 (compiled with Erlang/OTP 27)
➜  ~ elixir -e "IO.puts(System.tmp_dir!())"
/tmp
➜  ~ ls -l /tmp/mix_*
zsh: no matches found: /tmp/mix_*
➜  ~ mix new project1 && cd project1      
* creating README.md
* creating .formatter.exs
* creating .gitignore
* creating mix.exs
* creating lib
* creating lib/project1.ex
* creating test
* creating test/test_helper.exs
* creating test/project1_test.exs

Your Mix project was created successfully.
You can use "mix" to compile it, test it, and more:

    cd project1
    mix test

Run "mix help" for more commands.
➜  project1 mix compile
Compiling 1 file (.ex)
Generated project1 app
➜  project1 ls -l /tmp/mix_*                      
/tmp/mix_lock:
total 0
drwxr-xr-x  4 jie_old  wheel  128 Jan 10 21:03 tARo6JLGiDoBf2MhoMm72g

/tmp/mix_pubsub:
total 0
drwxr-xr-x  3 jie_old  wheel  96 Jan 10 21:03 tARo6JLGiDoBf2MhoMm72g
➜  project1 
➜  project1 su - jie
Password:
➜  ~ whoami
jie
➜  ~ elixir -v                             
Erlang/OTP 27 [erts-15.0] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit]

Elixir 1.18.1 (compiled with Erlang/OTP 27)
➜  ~ elixir -e "IO.puts(System.tmp_dir!())"
/tmp
➜  ~ mix new project2 && cd project2
* creating README.md
* creating .formatter.exs
* creating .gitignore
* creating mix.exs
* creating lib
* creating lib/project2.ex
* creating test
* creating test/test_helper.exs
* creating test/project2_test.exs

Your Mix project was created successfully.
You can use "mix" to compile it, test it, and more:

    cd project2
    mix test

Run "mix help" for more commands.
➜  project2 mix compile

21:04:17.346 [error] GenServer Mix.Sync.PubSub terminating
** (File.Error) could not make directory (with -p) "/tmp/mix_pubsub/VNbrPlEkR_STXjO2E53Uug": permission denied
    (elixir 1.18.1) lib/file.ex:346: File.mkdir_p!/1
    (mix 1.18.1) lib/mix/sync/pubsub.ex:222: Mix.Sync.PubSub.create_subscription_file/2
    (mix 1.18.1) lib/mix/sync/pubsub.ex:142: Mix.Sync.PubSub.handle_call/3
    (stdlib 6.0) gen_server.erl:2209: :gen_server.try_handle_call/4
    (stdlib 6.0) gen_server.erl:2238: :gen_server.handle_msg/6
    (stdlib 6.0) proc_lib.erl:329: :proc_lib.init_p_do_apply/3
Last message (from Mix.PubSub.Subscriber): {:subscribe, #PID<0.110.0>, "/Users/jie/project2/_build/dev"}
State: %{port: nil, hash_to_pids: %{}}
Client Mix.PubSub.Subscriber is alive

    (stdlib 6.0) gen.erl:241: :gen.do_call/4
    (elixir 1.18.1) lib/gen_server.ex:1125: GenServer.call/3
    (mix 1.18.1) lib/mix/sync/pubsub.ex:44: Mix.Sync.PubSub.subscribe/1
    (mix 1.18.1) lib/mix/pubsub/subscriber.ex:21: Mix.PubSub.Subscriber.init/1
    (stdlib 6.0) gen_server.erl:2057: :gen_server.init_it/2
    (stdlib 6.0) gen_server.erl:2012: :gen_server.init_it/6
    (stdlib 6.0) proc_lib.erl:329: :proc_lib.init_p_do_apply/3

21:04:17.355 [notice] Application mix exited: shutdown
** (ArgumentError) errors were found at the given arguments:

  * 1st argument: the table identifier does not refer to an existing ETS table

    (stdlib 6.0) :ets.lookup(Mix.State, :debug)
    (mix 1.18.1) lib/mix/state.ex:26: Mix.State.get/2
    (mix 1.18.1) lib/mix/cli.ex:112: Mix.CLI.run_task/2
    /Users/jie/.asdf/installs/elixir/1.18.1-otp-27/bin/mix:2: (file)

@jiegillet
Copy link
Contributor Author

elixir-lang/elixir#14168

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