harden(install-cli): chmod 700 the job-cache dir#29
Conversation
Follow-up to #28 review. The mktemp fallback already created a 0700 dir; the RUNNER_TEMP path used mkdir -p (umask-dependent, typically 0755). Lock the cache dir to owner-only on both paths so the binary we exec can't be read/replaced by another user on a shared self-hosted host. (The other #28 review comments were false positives under set -euo pipefail / the mv+rm cleanup — rebutted on the threads.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Code Review
This pull request updates the scripts/install-cli.sh script to explicitly set the permissions of the temporary binary directory to owner-only (700) using chmod, matching the fallback behavior of mktemp. There are no review comments, and I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
🅵 Fast automated pass — SAST + a quick, diff-scoped look (no repo-wide retrieval). For a deeper, repo-aware review, mention @lightbridge-assistant on this PR.
The diff is a clean, correct security hardening change. chmod 700 "$bin_dir" is applied immediately after mkdir -p, setting owner-only permissions that intentionally match the mktemp -d fallback's default 0700 behavior on line 63 — making both code paths consistent. Nothing blocking.
🤖 AI-generated review — treat it as untrusted, verify before acting; a human owns the final decision (AI governance).
Follow-up to #28 review (which was merged before the bot comments landed).
Applied: gemini's suggestion to lock the cache dir to 0700 — the mktemp fallback already did this; this makes the RUNNER_TEMP path match.
Rebutted on the #28 threads (false positives):
mktemp || exit 1/mv || exit 1:set -euo pipefailalready aborts on a failed $(…) assignment and before the trailing echo..downloadwas mv'd toflutter-toolsand.sha256sumsrm'd — the dir holds only the binary.🤖 Generated with Claude Code