Repository navigation
fix: pass values to Python as arguments and keep temporary files private - #173
Merged
Merged
Conversation
CybotTM
marked this pull request as ready for review
October 7, 2026 07:26
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 3 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (19)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
CybotTM
force-pushed
the
fix/secfix-hardening
branch
from
October 7, 2026 07:44
5df835b to
8b093cd
Compare
config_get_auto_update, config_get_global_auto_upgrade and check_manager_outdated built their Python program by pasting shell values into the program text. They now run a fixed program from a quoted heredoc and read the tool name, the project root and the snapshot path from sys.argv, so any value is looked up as a plain string. Assisted-by: claude-code:claude-opus-5-5 Agent-Session: https://claude.ai/code/session_018YeAfZRDbYWzRMVUPmZDXj Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
collect_endoflife now keeps a cycle only when it has the shape of a release number (3.13, 24, 1.23), for the API response as well as the file cache and the offline cache. The cycle ends up in tool keys, binary names and installer environment values. Assisted-by: claude-code:claude-opus-5-5 Agent-Session: https://claude.ai/code/session_018YeAfZRDbYWzRMVUPmZDXj Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
github_release_binary.sh, docker_plugin.sh, install_docker.sh and
install_claude.sh now download into a directory that mktemp -d creates
for the run (mode 0700, under $TMPDIR) and remove it when they finish.
Before, they used fixed or PID-derived names directly in /tmp.
The markers that installers leave for guide.sh (<tool>.already-current,
<tool>.held-back) now default to ${XDG_CACHE_HOME:-~/.cache}/cli-audit/markers
instead of /tmp/.cli-audit; CLI_AUDIT_MARKER_DIR still overrides it.
Assisted-by: claude-code:claude-opus-5-5
Agent-Session: https://claude.ai/code/session_018YeAfZRDbYWzRMVUPmZDXj
Agent-Host: 0493f0
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
generate_rollback_script now creates a directory of its own with tempfile.mkdtemp (mode 0700) and creates the script there with O_CREAT | O_EXCL | O_NOFOLLOW and mode 0700, instead of opening a timestamp-named file directly in the temp directory and making it world-readable. Tool names are written as single shell words (shlex.quote), and as repr() in comments. The rollback tests remove only a directory that generate_rollback_script created. Assisted-by: claude-code:claude-opus-5-5 Agent-Session: https://claude.ai/code/session_018YeAfZRDbYWzRMVUPmZDXj Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Assisted-by: claude-code:claude-opus-5-5 Agent-Session: https://claude.ai/code/session_018YeAfZRDbYWzRMVUPmZDXj Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
bulk_install generates a rollback script whenever a test enables it, and those scripts stayed in the system temp directory after the run. An autouse fixture points tempfile at the test's tmp_path. Assisted-by: claude-code:claude-opus-5-5 Agent-Session: https://claude.ai/code/session_018YeAfZRDbYWzRMVUPmZDXj Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
set_auto_update.sh, which guide.sh runs when the user presses 'a', built its Python program from an unquoted heredoc. It now reads the config path, the tool key and the value from sys.argv under a quoted heredoc, like the readers in scripts/lib/config.sh. Assisted-by: claude-code:claude-opus-5-5 Agent-Session: https://claude.ai/code/session_018YeAfZRDbYWzRMVUPmZDXj Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
re.match with ^...$ accepts a final newline; fullmatch does not. Assisted-by: claude-code:claude-opus-5-5 Agent-Session: https://claude.ai/code/session_018YeAfZRDbYWzRMVUPmZDXj Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
The end-to-end case ran install_tool.sh without CLI_AUDIT_MARKER_DIR, so its marker went to the user's cache and the check looked in a directory the suite had already removed. The installer now writes into the suite's directory, which is removed after that case. Assisted-by: claude-code:claude-opus-5-5 Agent-Session: https://claude.ai/code/session_018YeAfZRDbYWzRMVUPmZDXj Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Assisted-by: claude-code:claude-opus-5-5 Agent-Session: https://claude.ai/code/session_018YeAfZRDbYWzRMVUPmZDXj Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
The convenience-script downloads in install_docker.sh and install_claude.sh now pass --proto '=https' --proto-redir '=https' to curl and --https-only to wget, as github_release_binary.sh already does. Assisted-by: claude-code:claude-opus-5-5 Agent-Session: https://claude.ai/code/session_018YeAfZRDbYWzRMVUPmZDXj Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
CybotTM
force-pushed
the
fix/secfix-hardening
branch
from
October 7, 2026 07:51
8b093cd to
1b3688a
Compare
install_claude.sh drops its wget fallback for the native installer: the URL redirects, and only curl can restrict every redirect hop to HTTPS (--proto-redir). Without curl the script reports it and moves on to the Homebrew and npm methods, as it did when neither tool was present. The non-WSL path of install_docker.sh now passes --proto '=https' --proto-redir '=https' like the WSL path. Assisted-by: claude-code:claude-opus-5-5 Agent-Session: https://claude.ai/code/session_018YeAfZRDbYWzRMVUPmZDXj Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Assisted-by: claude-code:claude-opus-5-5 Agent-Session: https://claude.ai/code/session_018YeAfZRDbYWzRMVUPmZDXj Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
upgrade_claude migrates an npm installation to the native installer by removing the npm packages first. migrate_npm_to_native now checks for curl, which the native installer needs, before it removes anything, and otherwise keeps the npm installation and reports why. Assisted-by: claude-code:claude-opus-5-5 Agent-Session: https://claude.ai/code/session_018YeAfZRDbYWzRMVUPmZDXj Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
|
Member
Author
|
Review note for head 668c17b: CodeRabbit is rate-limited on this head and Copilot is out of review quota, so the review was done by six independent reviewer passes, each with a bounded finding criterion (false claims, reproducible defects, repository rules, unfixed weaknesses).
All checks pass, 0 unresolved threads, 0 open code-scanning alerts. Assisted by claude-code:claude-opus-5-5 — Session |
Member
Author
|
Self-review: 668c17b The bot review this pull request demands is unsatisfiable (Copilot quota wall or repeated bot failures on this head). The diff on this head was reviewed by the PR author; this comment is the on-the-record attestation the merge gate reads back. It stops matching on the next push. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merging this changes how the shell helpers hand values to Python, which endoflife.date cycles the collector accepts, where installers and the rollback generator keep temporary files, and how the Docker and Claude Code installer scripts are downloaded (curl held to HTTPS on every redirect). One behaviour changes for users: without curl,
install_claude.shno longer downloads the native installer with wget.installthen falls through to Homebrew or npm;upgradeof an existing native installation fails without changing it, andupgradeof an npm installation keeps it and reports that curl is required. Everything else behaves as before for valid input.Summary
scripts/lib/config.sh(config_get_auto_update,config_get_global_auto_upgrade),scripts/auto_update.sh(check_manager_outdated) andscripts/set_auto_update.shrun a fixed Python program from a quoted heredoc and pass the tool name, project root, config path, value and snapshot path as arguments.cli_audit/collectors.py:collect_endoflifekeeps a cycle only when the whole string has the shape of a release number (3.13,24,1.23), for the API response, the file cache and the offline cache. All cycles endoflife.date publishes today for go, nodejs, php, python and ruby have that shape.github_release_binary.sh,docker_plugin.sh,install_docker.shandinstall_claude.shdownload into a directory thatmktemp -dcreates for the run (mode 0700, under$TMPDIR) and remove it afterwards. Installer markers formake upgradedefault to${XDG_CACHE_HOME:-~/.cache}/cli-audit/markers;CLI_AUDIT_MARKER_DIRstill overrides it. The convenience-script downloads ininstall_docker.shandinstall_claude.shrun curl with--proto '=https' --proto-redir '=https', so every redirect stays on HTTPS;install_claude.shno longer falls back to wget for the native installer (wget cannot restrict redirects) and without curl moves on to the Homebrew and npm methods.cli_audit/bulk.py:generate_rollback_scriptwrites into a directory fromtempfile.mkdtemp, creates the script withO_EXCL/O_NOFOLLOWand mode 0700, and quotes tool names withshlex.quote.test_bulk.pyandintegration/test_e2e_install.pypointtempfileat each test'stmp_path, so generated rollback scripts no longer stay in the system temp directory. The installer run intest_guide_multi_install.shwrites its marker into the suite's own directory.CHANGELOG.md: a "Security" section under Unreleased.Linked issues / PRs
Builds on #169.
Type of change
fix:)Test plan
New tests. The ones marked (red) fail on the code before this change and pass here; the others pin behaviour that was already correct and must stay so:
tests/test_config_sh.py: a tool name with quotes and parentheses is looked up as a name (red);set_auto_update.shstores such a key as written (red); per-cycle keys and the global preference are read, and a key written byset_auto_update.shis found by the reader.tests/test_endoflife_cycles.py: other shapes, a trailing newline included, are dropped from the API response and from the file cache (red); release-number cycles pass.tests/test_github_release_binary.py: the download lands in a 0700 directory under$TMPDIRthat is removed afterwards; the already-current marker is written under~/.cache/cli-audit/markers(both red).tests/test_installer_workdirs.py: the same fordocker_plugin.sh,install_docker.sh(WSL path) andinstall_claude.sh(red);install_claude.sh upgradewithout curl keeps an npm installation (red against the previous head of this branch, which removed it).tests/test_bulk.py: the rollback script and its directory are owner-only, each call gets its own directory, tool names are single shell words (red).Gates on the pushed head:
uv run pytest tests/(1085 passed, 2 skipped)uv run python -m flake8 cli_audit tests,black --check,isort --check-only,mypy cli_auditbash tests/test_guide_multi_install.sh(52 passed),bash tests/test_reconcile_dryrun.sh(7 passed)./scripts/test_smoke.shshellcheck -x --severity=warningon every changed scriptChecklist
type(scope): description)git commit -S --signoff)CHANGELOG.mdupdated for user-visible changesAGENTS.mdfiles updated if structure / commands changed (none)Assisted by claude-code:claude-opus-5-5 — Session