[release-0.0.99.4] Prepare stable branch #1744
Conversation
Summary of ChangesHello @debarshiray, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on preparing the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Detected by https://www.shellcheck.net/: Line 1255: if ! localtime_target=$(readlink /etc/localtime >/dev/null 2>&3) \ ^-- SC2327 (warning): This command substitution will be empty because the command's output gets redirected away. ^-- SC2328 (error): This redirection takes output away from the command substitution. See: https://www.shellcheck.net/wiki/SC2327 https://www.shellcheck.net/wiki/SC2328 Fallout from 8db414d containers#1701 containers#1739 containers#1744 (cherry picked from commit d32dd5d) (cherry picked from commit a67f614)
debarshiray
force-pushed
the
wip/rishi/prepare-release-0.0.99.4
branch
from
313f0fc to
014b5ef
Compare
![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 prepares a stable branch for a new release. It includes a critical security fix for GHSA-4f99-4q7p-p3gh, which I've confirmed is correctly implemented. The vulnerability in the toolbox script that could allow writing to arbitrary host paths is now resolved. Additionally, I've reviewed the updates to the BATS test files. While they correctly adapt the tests for different dependency versions, I've suggested some minor improvements to enhance the clarity of test failure messages. Overall, this is a solid update, with the security fix being the most important part.
I am having trouble creating individual review comments. Click here to see my feedback.
toolbox (1255)
Redirecting readlink's stdout to /dev/null causes the localtime_target variable to always be empty. This makes the following check always true, leading to an unconditional and potentially insecure symlink creation (GHSA-4f99-4q7p-p3gh). The command substitution should capture the output of readlink.
test/system/102-list.bats (74-78)
The assert command provides a cryptic failure message (e.g., [ 1 -eq 2 ] failed). Using assert_equal from bats-assert would provide a more descriptive failure message if the assertion fails, improving test clarity.
if check_bats_version 1.10.0; then
assert_equal "${#lines[@]}" 2
else
assert_equal "${#lines[@]}" 3
fi
test/system/104-run.bats (359-366)
The else block is intended to fail the test when the number of lines in stderr is not 2 or 3. Using fail with a descriptive message would be more explicit and provide clearer output on failure than the current assert command.
if [ ${#stderr_lines[@]} -eq 2 ]; then
assert_line --index 1 "Error: failed to invoke command /etc in container $(get_latest_container_name)"
elif [ ${#stderr_lines[@]} -eq 3 ]; then
assert_line --index 1 "bash: line 1: exec: /etc: cannot execute: Is a directory"
assert_line --index 2 "Error: failed to invoke command /etc in container $(get_latest_container_name)"
else
fail "Expected 2 or 3 lines in stderr, but got ${#stderr_lines[@]}"
fi
|
Build failed. ✔️ unit-test SUCCESS in 1m 45s |
Until Bats 1.10.0, 'run --keep-empty-lines' had a bug where it counted
the trailing newline on the last line as a separate line [1]. However,
Bats 1.10.0 is only available in Fedora >= 39 and is absent from Fedoras
37 and 38.
[1] Bats commit 6648e2143bffb933
bats-core/bats-core@6648e2143bffb933
bats-core/bats-core#708
containers#1352
containers#1744
(backported from commit f716b23)
|
Build failed. ✔️ unit-test SUCCESS in 1m 46s |
Until Bats 1.10.0, 'run --keep-empty-lines' had a bug where it counted
the trailing newline on the last line as a separate line [1]. However,
Bats 1.10.0 is only available in Fedora >= 39 and is absent from Fedoras
37 and 38.
[1] Bats commit 6648e2143bffb933
bats-core/bats-core@6648e2143bffb933
bats-core/bats-core#708
containers#1352
containers#1387
containers#1744
(backported from commits f716b23 and
7abfa70)
Bash 5.3.0 changed the error messages shown by its exec built-in [1].
With Bash 5.2.37:
$ exec /etc
bash: /etc: Is a directory
bash: exec: /etc: cannot execute: Is a directory
With Bash 5.3.0:
$ exec /etc
bash: /etc: Is a directory
The 'assert' function cannot directly handle compound commands. So,
those need to be wrapped in 'bash -c "..."' [2].
[1] Bash commit b8c60bc9ca365f82
See how exec_builtin() handles EX_NOEXEC and EISDIR from
shell_execve() to avoid printing a duplicate error message.
https://cgit.git.savannah.gnu.org/cgit/bash.git/commit/?id=b8c60bc9ca365f82
[2] https://github.com/bats-core/bats-assert
containers#1688
containers#1699
containers#1739
containers#1744
(backported from commit 6c98db6)
(cherry picked from commit 0090893)
debarshiray
force-pushed
the
wip/rishi/prepare-release-0.0.99.4
branch
from
881a86d to
9a24ed3
Compare
containers#1265 containers#1744 (cherry picked from commit 6b6cb1b)
Fedora 36 reached End of Life on 16th May 2023: https://docs.fedoraproject.org/en-US/releases/eol/ containers#1313 containers#1744 (cherry picked from commit 0676eb9)
containers#1385 containers#1744 (cherry picked from commit 53a0316)
|
Build failed. ✔️ unit-test SUCCESS in 1m 51s |
With the recent expansion of the test suite, it's necessary to increase the timeout for all Fedora nodes to prevent the CI from timing out. containers#1387 containers#1744 (cherry picked from commit b8138e0)
Fedora 37 reached End of Life on 15th November 2023: https://docs.fedoraproject.org/en-US/releases/eol/ containers#1418 containers#1744 (cherry picked from commit 9c2b5e9)
|
Build failed. ✔️ unit-test SUCCESS in 1m 56s |
With the recent expansion of the test suite, it's necessary to increase the timeout for the Fedora 38 and 39 nodes to prevent the CI from timing out. containers#1445 containers#1739 containers#1744 (cherry picked from commit ce66b0b) (cherry picked from commit 754feea)
Stable branches for old Toolbx releases are only about the toolbox(1) executable. It doesn't make sense to have old "stable" versions of the image definitions, because the registries will have only one set of images built from the definitions in the main branch, and all supported versions of toolbox(1) must work with them. containers#1739 containers#1744 (cherry picked from commit f2b2a18)
|
Build failed. ✔️ unit-test SUCCESS in 1m 52s |
|
Build failed. ✔️ unit-test SUCCESS in 2m 32s |
|
Build failed. ✔️ unit-test SUCCESS in 1m 59s |
|
Build failed. ✔️ unit-test SUCCESS in 2m 01s |
|
Build failed. ✔️ unit-test SUCCESS in 2m 00s |
This is meant to make the project more searchable on the Internet. More and more people have been pointing out that "toolbox" is terribly difficult to search for, and it's impossible to find any decent Internet real estate by that name. containers#1399 containers#1740 containers#1744 (backported from commit c3403da) (backported from commit e7043ea)
|
Build failed. ✔️ unit-test SUCCESS in 1m 48s |
|
Build failed. ✔️ unit-test SUCCESS in 1m 50s |
|
Build failed. ✔️ unit-test SUCCESS in 1m 55s |
2 participants
... for CVE-2025-65637 or GHSA-4f99-4q7p-p3gh.
https://github.com/containers/toolbox/security/dependabot/26