Handle uv self-update failures from external package managers#235
Conversation
… manager `uv self update` exits non-zero when uv was installed through Homebrew/apt (it can't replace a binary it doesn't own). With `set -e` that aborted the whole installer before the CLI was ever installed. Swallow the failure and proceed — a package-managed uv is kept current by its manager anyway. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L7SAdcQr3ifEiHQSz1jdS7
Installer selection now prefers an existing uv, falls back to an existing pipx, and only bootstraps uv when neither is present (instead of always assuming uv). System-dependency handling now installs the missing optional deps via Homebrew when `brew` is available — but only the formulae brew actually carries (guarded by `brew info`), so an unknown formula can't fail the batch. Anything brew can't provide falls through to the existing platform-specific install advice, which is also used when brew is absent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L7SAdcQr3ifEiHQSz1jdS7
![aikido-pr-checks[bot]](https://avatars.githubusercontent.com/in/898896?s=60&v=4){kind=small}
| # manager (Homebrew, apt, …) — it can't replace a binary it doesn't own. That | ||
| # is not fatal to us: a managed uv is already kept current by its manager, so | ||
| # swallow the failure and proceed straight to installing the CLI. | ||
| uv self update 2>/dev/null || true |
There was a problem hiding this comment.
uv self update 2>/dev/null || true suppresses errors and output; consider logging failures instead of swallowing them to avoid hiding unexpected or malicious behavior.
Details
✨ AI Reasoning
The change intentionally redirects or swallows error output for operations that execute external code (uv self update and brew install). Silencing stderr and forcing a successful exit code removes visible failure signals and can hide unexpected or malicious behavior from reviewers and users. This reduces transparency of what the installer actually did and makes post-failure investigation harder.
🔧 How do I fix it?
Ensure code is transparent and not intentionally obfuscated. Avoid hiding functionality from code review. Focus on intent and deception, not specific patterns.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
| # ones it actually carries (brew needs no sudo); for anything left we print how to | ||
| # install it — without touching the system or invoking sudo on the user's behalf. |
There was a problem hiding this comment.
The install_system_deps description says it works “without touching the system,” but the same function runs brew install, so the stated behavior contradicts actual execution.
| # ones it actually carries (brew needs no sudo); for anything left we print how to | |
| # install it — without touching the system or invoking sudo on the user's behalf. | |
| # ones it actually carries (brew needs no sudo); for anything Homebrew can't provide | |
| # we print how to install it without touching the system or invoking sudo on the user's behalf. |
Details
✨ AI Reasoning
The updated dependency-install path now performs package installation through Homebrew. However, the accompanying explanatory text still states that this path operates without modifying the system. Those two statements cannot both be true at runtime, so the documented assumption is impossible to satisfy given the control flow.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
install.sh: - Add a development install mode: --install-method git (aliases --dev/-e/ --editable/--git), with --dir and AAI_INSTALL_METHOD/AAI_GIT_DIR env knobs and --help. Dev mode reuses the checkout it runs from (or clones the repo) and installs it editable: `uv tool install -U -e <path>` (or `pipx install --force -e <path>`). - Installer selection prefers uv, then pipx, then bootstraps uv. - System deps: brew-install only the formulae Homebrew actually carries (guarded by `brew info`), falling back to printed advice otherwise. ci.yml: new "install script smoke" job runs ./install.sh --install-method git (editable install of the PR checkout via uv) and smoke-tests `assembly --version` / `assembly --help`. tests/test_code_tui.py: harden the flaky spinner test (stop the live interval timer and pause before asserting the pinned elapsed readout) that was failing CI. README.md: document dev mode; switch the install one-liner to bash. Note: pushed with the gate bypassed at the user's request. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L7SAdcQr3ifEiHQSz1jdS7
| prepare_git_source() { | ||
| if [ -f pyproject.toml ] && grep -q '^name = "aai-cli"' pyproject.toml 2>/dev/null; then | ||
| PACKAGE="$(pwd)" | ||
| echo "Development install from current checkout: $PACKAGE" |
There was a problem hiding this comment.
Echo prints the user-derived PACKAGE value (path/URL). Avoid logging raw user-controlled paths; consider redacting or omitting sensitive details.
Details
✨ AI Reasoning
The change adds multiple echo/logging statements that output values originating from user-controllable sources: PACKAGE may be set to the current working directory or a clone path, and GIT_DIR can be provided via environment or CLI flags. Printing these paths can expose filesystem locations or other personal data. This is a newly introduced behavior in the changed code and increases risk of leaking user-controlled/personal data to stdout.
🔧 How do I fix it?
Keep sensitive data such as emails, passwords, and tokens out of logs. When logging values tied to a user, prefer a safe identifier like a user ID over the raw input, and strip line breaks from any user-provided text you do log.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
The previous hardening added `await pilot.pause()` after stopping the spin timer, which deadlocks pilot — the test hung and wedged the whole pytest job until CI's 15-minute timeout cancelled it (failing the run). Stopping the live interval timer already removes the real-time tick that raced the assertion, and Static.update()->render() is synchronous, so the pause was both unnecessary and harmful. Drop it. Full suite passes with no hang. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L7SAdcQr3ifEiHQSz1jdS7
2 participants
Summary
Make the
install.shscript resilient touv self updatefailures when uv is installed via external package managers like Homebrew or apt, which cannot replace binaries they don't own.Changes
uv self updatewith error suppression (2>/dev/null || true) ininstall.shImplementation Details
The change recognizes that when uv is installed via a system package manager, it cannot update itself (the package manager owns the binary). This is not a fatal condition since the package manager is responsible for keeping uv current. By suppressing the error and continuing, the script accommodates both self-managed uv installations (which can self-update) and externally-managed ones (which cannot).
https://claude.ai/code/session_01L7SAdcQr3ifEiHQSz1jdS7