Repository navigation
fix(installer): fail closed on missing Node.js integrity evidence - #861
Conversation
Both installers verify the private Node.js runtime against the release's SHASUMS256.txt, but each let the download through when the check could not be made: - install.sh warned and extracted the tarball when SHASUMS256.txt could not be fetched, did not list the package, or neither sha256sum nor shasum was on PATH. - install.ps1 did the same for an unreachable or unlisted checksum, and for a digest mismatch too: Fail raises under the script's ErrorActionPreference, so the catch around the checksum block caught it and printed "continuing". Each of those cases now stops before extraction with its own reason and a hint to install Node.js >= 22 with npm and re-run. The PowerShell checksum fetch also passes -UseBasicParsing, so Windows PowerShell 5.1 without the IE engine is not refused over a parser. The optional Chinese font for deck preview had the same gap: with no hash tool, an empty digest counted as a match. It now compares the digest directly, as the LibreOffice download already did, so an unchecked face is skipped with the existing warning. Tests run the real provision_private_node, latest_node_v22 and sha256_of from install.sh, and the real Install-PrivateNode and Fail from install.ps1 under pwsh (skipped where pwsh is absent), against a fake nodejs.org that serves one release at exact URLs. Addresses requirement 1 of WenyuChiou/awesome-agentic-ai-zh#289 once this commit ships in a tagged release. Co-authored-by: Claude (claude-opus-5-5[1m]) <noreply@anthropic.com>
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
I reviewed the refreshed github/main...HEAD diff, the direct and fallback callers of private Node provisioning on both platforms, and the introducing history. The new paths fail before extraction when checksum evidence is unavailable, absent, or mismatched; the PowerShell catch boundary also no longer swallows Fail. I found no weakened tests or backward-compatibility regression.
Coverage included the repository rules (AGENTS.md and its CLAUDE.md alias), the full diff, callers, history, compatibility, test quality, and architecture constraints; the runtime/TUI domain boundaries are not implicated by these installer-only changes.
Verification: uv run pytest tests/test_install_script.py -x passed with 51 passed and 4 skipped. The skipped cases require PowerShell, which is unavailable in this environment, so I reviewed that path statically. sh -n install.sh, the source-language checker, Ruff check and format check for the changed test file, and git diff --check all passed. The initial test attempt stopped in an unrelated autouse fixture because the fresh environment lacked the workspace plugin; after uv sync --all-packages, the unchanged test command passed.
ZuyiZhou
left a comment
There was a problem hiding this comment.
Approved at 902e351. Read the diff: both installers now stop before extraction when SHASUMS256.txt cannot be fetched, does not list the package, or no hash tool is present; the PowerShell mismatch Fail now sits outside the try/catch; the font step compares the digest directly. install.sh at this head passes sh -n and bash -n. All CI checks are green at this head. Not rerun here: the test suite, and install.ps1 on Windows (the description says so too).
## Summary Native Windows installs fail from the shell Windows ships with. Both commands the README gives for Windows break under Windows PowerShell 5.1: - `irm https://raven.evermind.ai/install.ps1 | iex` stops with `(308) Permanent Redirect`. PowerShell 5.1 follows 301, 302 and 307 redirects but not 308, which is what that URL answers with. This is the error #148 reported; the README already offered the direct URL for it. - The direct URL downloads the script, which then stops at "Could not resolve the latest Raven release wheel from GitHub". `Resolve-RavenLatestVersion` requests the release page without following the redirect and reads the Location header off the exception that raises. Only PowerShell 7 attaches the response to that exception. PowerShell 5.1 returns the unfollowed redirect as the response and reports `MaximumRedirectExceeded` as an error with no response, so the lookup always came back empty. This came in with #299 and has been on main since. Changes: - `install.ps1`: the release-page request uses `-ErrorAction Ignore` instead of `-ErrorAction Stop`, so PowerShell 5.1 reads Location from the returned response. PowerShell 7 still raises regardless of `-ErrorAction` and goes through the existing catch. The Location is still matched against the same stable-tag pattern. - CI: the installer job ran `install.ps1` under `pwsh` only, which is how this shipped. It now also runs the piped install under Windows PowerShell 5.1 (`shell: powershell`), into its own tool, bin and home directories, sharing the uv cache. - Docs: README, README.zh-CN, the docs-site quick start (en, zh) and the release notes template now say that Windows PowerShell 5.1 stops on the short URL with `(308) Permanent Redirect` and to switch to the direct URL when that appears. The short URL's redirect itself is unchanged. - Tests: tripwires for the lookup (`-ErrorAction Ignore`, catch kept) and for the CI gate (both shells, and separate tool, bin and home directories, since the version check runs the `raven.exe` in the bin directory). Second topic, Windows test hygiene: three tests failed when the suite ran on a Chinese-locale Windows checkout. CI runs the unit tests on Linux only, so it never saw them. - `test_no_workflow_step_enters_the_removed_page_directory` read workflows in the locale encoding (GBK there) and stopped on the UTF-8 in `release.yml`. It now reads UTF-8. - The font and LibreOffice harness tests run `install.sh` code under `sh`, which on Windows is Git Bash: its curl sits outside `/usr/bin` and paths mix separators, so two failed and others passed for the wrong reason (the digest-mismatch test passed because curl was missing). The ten harness tests now carry the POSIX-only skip the other three sh-driven tests already had, shared as one `POSIX_SH_ONLY` marker. Linux runs are unchanged. #861 merged first, and this branch is rebased onto it. The one conflict was in `tests/test_install_script.py`. The sh-driven tests #861 added, its two Node harness tests and `test_a_download_that_cannot_be_hashed_is_not_installed`, now carry `@POSIX_SH_ONLY` like their neighbours, so the marker stays the one spelling of that skip. Third topic, test infrastructure: Python's locale-dependent text I/O uses the system code page when no `encoding=` is passed. On a GBK-locale Windows checkout, any test that reads or writes a file containing non-ASCII bytes will fail or silently corrupt data. The class of bug was found above; there are ~790 `read_text()` call sites without `encoding` in `tests/`. Measured on Linux under a zh_CN.GBK locale, main without UTF-8 mode is not one test short: 87 tests fail only there, across 25 files, and `tests/test_rpc_schema_match.py` (418 tests) does not collect, mostly on `UnicodeDecodeError` from reading UTF-8 files. - Makefile: `export PYTHONUTF8 := 1`, so every recipe runs in UTF-8 mode, `check-core-wheel` (part of `make ci`) included. - `tests/conftest.py` raises at import time on any interpreter whose default text encoding is not the UTF-8 codec, telling the caller to set `PYTHONUTF8=1` or use `-X utf8`. It compares codecs rather than names, so Windows with the system-wide UTF-8 option, which reports `cp65001`, passes. `uv run pytest`, which AGENTS.md documents and which does not read the Makefile, now stops here on a GBK checkout, as does an IDE runner that does not set the variable. CI's `--noconftest` self-upgrade job (its tests all name `encoding` explicitly) is unaffected. `tests/test_conftest_utf8_guard.py` pins the codec comparison and drives the refusal in an ASCII-locale child. - CI sets `PYTHONUTF8` in no job. The jobs that load `tests/conftest.py` run on ubuntu-latest, where the locale is UTF-8 already, and the `unit` job gets the variable from the Makefile through `make coverage-shard`. The Windows self-upgrade job runs its one file with `--noconftest`, and that file names its encodings. The installer job runs no tests at all: its steps are raven installing itself and answering `--version`, so the variable there would only change how raven runs, and that gate exists to run it the way users do. ## Type - [x] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification Run on Windows 11 (zh-CN) with Windows PowerShell 5.1.26100 and PowerShell 7.6.6, before the rebase onto #861 and the second review round. Every install below was isolated: `UV_TOOL_DIR`, `UV_TOOL_BIN_DIR` and `RAVEN_HOME` in a temp directory, `RAVEN_MINIMAL=1`, `RAVEN_NO_LAUNCH=1`. - Before the fix, under 5.1: `irm https://raven.evermind.ai/install.ps1` fails with `(308) Permanent Redirect`; `irm https://raw.githubusercontent.com/EverMind-AI/Raven/refs/heads/main/install.ps1 | iex` stops at `Could not resolve the latest Raven release wheel from GitHub`. - `Invoke-WebRequest https://github.com/EverMind-AI/Raven/releases/latest -MaximumRedirection 0 -UseBasicParsing -ErrorAction Stop`: 5.1 throws `InvalidOperationException` (`MaximumRedirectExceeded`) with no `Response`; 7.6.6 throws `HttpResponseException` whose `Response.Headers.Location` is the tag URL. With `-ErrorAction Ignore`, 5.1 returns the 302 with Location set, and 7.6.6 still throws into the catch. - Redirects under 5.1, via httpbin `redirect-to`: 301, 302 and 307 are followed; 308 is not. - After the fix, `Get-Content install.ps1 -Raw | Invoke-Expression` under 5.1 and under 7.6.6, with `uv tool update-shell` disabled in a temp copy of the script to leave the user PATH alone: both resolved v0.2.4, installed raven with everos-memory, design-engine and ppt-engine, and `raven.exe --version` printed `Raven v0.2.4`. - `uv run pytest tests/test_install_script.py tests/test_release_plugin_list.py tests/test_constraints_export_contract.py` plus the six installer tests in `tests/test_cli_onboard_commands.py`, on Windows: 48 passed, 13 skipped (before: 55 passed, 3 skipped, 3 failed). The two new tripwires fail against the previous `install.ps1` and `ci.yml`. With the UTF-8 gate added, a bare `uv run pytest --co -q tests/test_install_script.py` on the GBK checkout raises RuntimeError naming `PYTHONUTF8=1`, while the same invocations with the variable set pass (45 passed, 13 skipped). - `uv run ruff check` and `uv run ruff format --check` on both test files pass; `ty` does not cover `tests/`, the only Python this changes. `PYTHONPATH=. uv run python scripts/check_source_language.py origin/main` exits 0. Run on Linux (Ubuntu 22.04, Python 3.12.13) at this head. A zh_CN.GBK locale compiled into a private `LOCPATH` gives a non-UTF-8 interpreter (`locale.getpreferredencoding(False)` is `GBK`, UTF-8 mode off). - Full suite, `uv run --frozen --python 3.12 --all-extras pytest -q`: 7 failed, 27567 passed, 119 skipped. On main the same command gives 7 failed, 27556 passed, 119 skipped with the identical 7 failing IDs, this machine's local baseline (five proxy tests, a root-only permission test and an npm found on PATH), so 0 introduced; the 11 extra passes are this PR's new tests. - Main under GBK without UTF-8 mode: 86 failed, 27059 passed, 119 skipped, 11 errors. 87 IDs fail only there, and `tests/test_rpc_schema_match.py` does not collect. One of the 87, a non-ASCII file name, is specific to Linux, where GBK also becomes the filesystem encoding. - This head under GBK: a bare `uv run pytest --co -q tests/test_install_script.py` stops at the conftest RuntimeError naming `PYTHONUTF8=1`. `make test-python`, which exports it: 7 failed, 27567 passed, 119 skipped, the same 7 IDs as under UTF-8. `make check-core-wheel`: 1 passed, where with the per-target exports it exited 2 at the guard. - The harness tests marked `@POSIX_SH_ONLY` run on Linux in the full suite above. - Mutations: with the old name tuple back in the guard's predicate, only the `cp65001` case of `tests/test_conftest_utf8_guard.py` fails; with the guard deleted, its end-to-end refusal test fails. Giving the 5.1 step pwsh's `UV_TOOL_BIN_DIR`, or its `RAVEN_HOME`, now fails the CI gate test naming the shared variable, where before it passed. - `make lint-python` passes, and the commit-message, source-language and large-file gates exit 0 over `origin/main..HEAD`. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [x] User-facing docs or screenshots are updated when needed ## Risk User-visible: the one-line Windows install works again from Windows PowerShell 5.1 through the direct URL. The short URL still answers with a 308 there, and the docs now name that error. `install.ps1` is served from main, so the fix reaches users on merge, and reverting the squash commit rolls it back the same way. The resolved Location is still matched against the same stable-tag pattern before anything is downloaded. The installer CI job gains one Windows install, which reuses the uv cache. Developer-visible: on a Windows checkout whose code page is not UTF-8, a pytest run that loads `tests/conftest.py` without UTF-8 mode stops at import with a message naming `PYTHONUTF8=1`; the `make` targets set it, and Linux and macOS runs are unaffected. On Windows the sh-driven harness tests report as skipped: the ten this PR marks and `test_a_download_that_cannot_be_hashed_is_not_installed`, beside the five that already skipped there. - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes ## Related Issues #148 --------- Co-authored-by: Claude (claude-opus-5-5) <noreply@anthropic.com>
Summary
Both installers verify the private Node.js runtime against the
release's SHASUMS256.txt before running it, but each let the download
through whenever the check could not be made. This makes every such
case stop before extraction.
Measured on
mainbefore this change, running the real functionsagainst a fake nodejs.org that serves one release at exact URLs:
After it, every row but the first stops with its own reason and a
hint to install Node.js >= 22 with npm and re-run; the installer then
uses that Node instead of downloading one.
install.sh: the three warn-and-continue branches becomedie,each removing the staged download first. Hashing goes through the
existing
sha256_ofhelper.install.ps1:Failraises under the script's$ErrorActionPreference = "Stop", and the mismatchFailsatinside the
trywhosecatchprinted "Could not verify Nodechecksum; continuing". Only the fetch stays inside that
trynow.The fetch also passes
-UseBasicParsing, so Windows PowerShell 5.1without the IE engine is refused for missing evidence only, never
for a parser it lacks.
(no hash tool) as a match. It now compares the digest directly, as
the LibreOffice download already did, so an unchecked face is
skipped with the existing warning and the install carries on.
Unchanged and out of scope: the uv bootstrap (astral.sh) and the raven
wheels are not hash-pinned, and SHASUMS256.txt comes from the same
origin as the tarball over HTTPS, so its signature is not checked.
Context: requirement 1 of the external review in
WenyuChiou/awesome-agentic-ai-zh#289.
Type
Verification
Run in a worktree. The head
902e351sits on3632e6040; the sixcommits
maingained while this branch was in flight touch none of thethree files it changes. The test-first and mutation runs below were
taken before that rebase, on
5be9698a1; the full suite and theinstaller test file (with pwsh) were re-run at the head.
Tests first, against the unmodified scripts: the first command gave
4 failed (install.sh unreachable, not listed, no hash tool; the font
with no hash tool) while the mismatch and matching cases passed. The
second gave 3 failed (mismatch, unreachable, not listed) and 1
passed. After the change both selections pass.
One-construct mutations of
install.shagainst the new tests:dropping the no-hash-tool refusal, the not-listed refusal, the
unreachable refusal, the staged-download cleanup, or the font's
direct comparison each turned exactly one test red; a control that
skips extraction turned the matching-digest test red.
The installer test file under
--idle-ceiling-strict: 54 passed,1 failed; without pwsh, 50 passed, 4 skipped, 1 failed. The failure
is
test_resolve_node_dir_answers_each_case_it_exists_for, whichfails the same way on the base on this machine: its fixture PATH
includes
/usr/bin, which holds an npm here. It stubsprovision_private_node, so this change cannot reach it.Full suite at the head, pwsh on PATH: 27560 passed, 7 failed, 115
skipped in 350.94 s. The seven are this machine's standing failures
on
main: five proxy tests intests/test_config_update_providers.py(the shell carries a proxy),the node-dir test above, and one that fails when run as root:
tests/test_subagent_node_runtime.py::test_what_cannot_be_read_names_nothingLive network: the extracted
install.shfunctions provisionedv22.23.3 from nodejs.org, verified it against the published
SHASUMS256.txt, ran it, and left no staged download, in 6 s.
dash -nandbash -noninstall.sh: clean. The PowerShellparser over
install.ps1on this branch and on the base: 0 errorseach.
ruff checkandruff format --checkon the test file:clean.
Not verified here:
install.ps1has not run on Windows, and-UseBasicParsingmatters only to Windows PowerShell 5.1, which notest exercises. CI does not close that gap: the two installer jobs
find Node v22.23.3 already on their runners, so neither reaches the
private-Node download this change touches.
Relevant tests pass locally
Relevant lint / type checks pass locally
User-facing docs or screenshots are updated when needed
No user-facing doc describes the checksum behaviour, so none changed.
Risk
A machine without Node.js >= 22 that cannot fetch the release's
SHASUMS256.txt, or that has neither sha256sum nor shasum, used to get
an unverified runtime with a warning; it now gets an error and the
hint above. A system Node >= 22 never downloads anything and is
unaffected. On the build-time path (a system node without npm) the
download was already subshelled or caught, so a refusal there skips
the TUI and page builds with the existing warning rather than ending
the install. The font step changes only on a machine with no hash
tool. Both scripts are served from
main, so the change reaches newinstalls at merge, and reverting this commit rolls it back the same
way.
Related Issues
N/A in this repository. External:
WenyuChiou/awesome-agentic-ai-zh#289 (requirement 1, once a tagged
release carries this).