exec: use default env with --env#2082
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements environment variable merging for the exec command, ensuring that variables provided at runtime are combined with the container's default environment instead of replacing it. A potential null pointer dereference was identified in src/libcrun/container.c where container->container_def->process is accessed without a safety check, which could lead to crashes in configurations where the process section is missing.
| return crun_make_error (err, errno, "putenv `%s`", process->env[i]); | ||
| } | ||
| else if (container->container_def->process->env_len) | ||
| if ((container->container_def->process->env_len) && (merge_env || !process->env_len)) |
There was a problem hiding this comment.
There is a potential null pointer dereference here. The code accesses container->container_def->process->env_len without verifying that container->container_def->process is not NULL. While a valid OCI configuration typically includes a process section, other parts of the codebase (e.g., lines 3794 and 4024) explicitly check for its existence, indicating it can be NULL in some contexts. This change makes the dereference unconditional when merge_env is true, which could lead to a crash with certain container configurations.
if (container->container_def->process && container->container_def->process->env_len && (merge_env || !process->env_len))There was a problem hiding this comment.
fixed (although this is a pre-existing issue)
|
TMT tests failed. @containers/packit-build please check. |
When crun exec --env is used, the environment from the container spec is not set (including PATH): crun exec env PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin TERM=xterm HOME=/root crun exec --env FOO=bar env executable file `env` not found in $PATH: No such file or directory crun exec --env FOO=bar /bin/env FOO=bar HOME=/root Fix this, and add a test. The modified test fails before the fix: $ RUN_TESTS=exec-add-environment_variable python3 tests/test_exec.py # crun command failed: /home/kir/git/crun/crun --root /home/kir/git/crun/.testsuite-run-325177/root exec --env FOO=BAR test-tmp7g12rkzv /init printenv PATH # Return code: 139 # Test 'exec-add-environment_variable' failed with exception after 0.132s: # Exception type: CalledProcessError .... Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1 participant
When crun exec --env is used, the environment from the container spec is not set (including PATH):
Fix this, and add a test.
The modified test fails before the fix:
Fixes: #2079