Skip to content

fix(update): stop reporting success when SQLite remains vulnerable - #95801

Closed
fangliquanflq wants to merge 3 commits into
NousResearch:mainfrom
fangliquanflq:fix/update-verify-sqlite-remediation
Closed

fangliquanflq wants to merge 3 commits into
NousResearch:mainfrom
fangliquanflq:fix/update-verify-sqlite-remediation

Conversation

@fangliquanflq

Copy link
Copy Markdown

What does this PR do?

hermes update now probes the Python interpreter that Hermes will use after the update before printing a success banner. If that interpreter still links a SQLite build with the WAL-reset corruption bug, the update is reported as partial, gateway status and update receipts stay partial, and the user gets an explicit uv-managed Python recovery path instead of a false success.

Symptom

A user can run hermes update in a Git install whose active venv links vulnerable SQLite, receive a successful completion message, restart Hermes, and still see the same WAL-reset warning and SQLite version.

Impact

Operators can believe the runtime remediation succeeded while Hermes continues using the vulnerable SQLite build. The affected report experienced malformed FTS indexes and later write failures before rebuilding the venv on a uv-managed Python.

Bug Cause

Trigger: hermes_cli/update_cmd.py / _cmd_update_impl current-checkout completion and _print_update_summary

Causal chain:

  1. A Git install runs under a venv outside the checkout and links a vulnerable SQLite build.
  2. The managed-runtime repair targets checkout-local venv or .venv, while both the current-checkout path and the normal update summary print success without probing the interpreter that the next Hermes launch will use.
  3. The command reports success even though the linked SQLite version is unchanged and the WAL-reset warning persists after restart.

Why it is wrong: Update completion was based on code, dependency, and Desktop outcomes, but it did not verify the specific runtime property that the remediation warning promised to repair.

Working sibling / contrast: Checkout-local managed venvs already have a transactional uv runtime replacement path that provisions and smoke-tests a SQLite-safe Python before cutover. The missing piece was outcome verification and truthful reporting when that path is not applicable or does not complete.

Ruled out: This is not caused by SQLite version parsing or the vulnerability predicate. Existing runtime tests correctly classify 3.46.1 as vulnerable and fixed/backported releases as safe; the missing check was at update completion.

Fix

  • Resolve the post-update interpreter from the checkout venv when present, otherwise use the running venv interpreter, and probe its linked SQLite in an isolated subprocess.
  • Gate both normal and already-current success banners on that probe.
  • Mark gateway update status and update receipts partial when runtime verification fails or still reports vulnerable SQLite.
  • Print an actionable recovery path that names a uv-managed Python and hermes doctor verification.

Related Issue

Fixes #95772

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Changes Made

  • hermes_cli/update_cmd.py - verify post-update SQLite runtime health before reporting success and propagate partial status to gateway/receipt outcomes.
  • tests/hermes_cli/test_update_sqlite_remediation.py - cover external venv selection, vulnerable-runtime reporting, and current-checkout completion.
  • tests/hermes_cli/test_update_desktop_stale_warning.py - keep Desktop summary tests explicit about unrelated SQLite health.
  • tests/hermes_cli/test_cmd_update.py - isolate existing update-flow tests from host SQLite variability.

How to Test

  1. Run the focused SQLite remediation, summary, runtime, and receipt tests.
  2. Run the current-checkout updater regression test.
scripts/run_tests.sh tests/hermes_cli/test_update_sqlite_remediation.py tests/hermes_cli/test_update_desktop_stale_warning.py tests/hermes_cli/test_sqlite_runtime.py tests/hermes_cli/test_update_receipt.py -q
scripts/run_tests.sh tests/hermes_cli/test_cmd_update.py -k 'update_on_fork_checks_upstream_when_origin_up_to_date' -q

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • I've run the repo test entry on the relevant tests and all selected tests pass
  • I've added tests for my changes
  • I've tested on my platform: Windows 11

Documentation & Housekeeping

  • Relevant documentation update: N/A - user-facing remediation is emitted by the command
  • cli-config.yaml.example update: N/A - no config keys changed
  • CONTRIBUTING.md or AGENTS.md update: N/A - no workflow changed
  • Cross-platform impact considered - interpreter resolution uses existing platform-aware venv helpers
  • Tool descriptions/schemas update: N/A - no model tool changed

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard area/install-update Installer, updater, packaging, wheels, doctor sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 26, 2026
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

fix(update): stop reporting success when SQLite remains vulnerable — verified completion

  • Runtime probe: hermes_cli/update_cmd.py:13-51 adds _post_update_sqlite_runtime_status() probing the post-update venv python (project_venv_dir → venv_python_path else sys.executable) via probe_sqlite_runtime, and _print_verified_update_completion() which withholds the ✓ banner and prints a ⚠ Update partially complete with version-specific WAL-reset guidance when vulnerable.
  • Summary integration: _print_update_summary:64-98 now includes sqlite_runtime_ok alongside node_failures/desktop_build_ok, aggregates parts, returns bool (desktop_build_ok and sqlite_runtime_ok). Call sites (_update_via_zip:107, _cmd_update_impl:201,764) propagate update_complete to gateway exit code and finalize_update_receipt("partial" | "success").
  • Current-checkout path: hermes_cli/update_cmd.py:153-193 tracks current_checkout_complete, uses verified completion for both venv-repair and node-repair branches, and sys.exit(1) with False exit code + partial receipt when verification fails. Tests tests/hermes_cli/test_update_sqlite_remediation.py and test_cmd_update.py:259-357 pin runtime probe, summary withholding, and durable failure for both python and node repair paths.
  • Non-blocking: probing runs twice (once in helper, once in summary) — cheap but could be memoized within the turn if perf matters.

Non-blocking — correct durable-outcome fix.

@teknium1

Copy link
Copy Markdown
Collaborator

Salvaged onto current origin/main (your branch was ~1000 commits behind and conflicted with the #97052 fork-completion and #91360 config-migration changes on the current-checkout path) as PR #99715 — all 3 of your commits cherry-picked with your authorship preserved, plus a contributors/emails/ mapping for attribution CI.

Evidence: tests/hermes_cli/test_update_sqlite_remediation.py + test_update_desktop_stale_warning.py → 12 passed; test_sqlite_runtime.py + test_update_receipt.py → 47 passed; the 6 test_cmd_update.py failures on the branch reproduce identically on a clean origin/main worktree (pre-existing, A/B verified).

This PR will be closed with credit once #99715 merges. Thanks — this was the closest and cleanest attempt at the #90906 "false success while SQLite stays 3.50.x" verification gap.

@teknium1

Copy link
Copy Markdown
Collaborator

Merged via PR #99715 — all 3 of your commits were cherry-picked onto current origin/main with your authorship preserved in git log, plus a contributors/emails/ attribution mapping.

#99715

Thanks — this was the closest and cleanest attempt at the #90906 "update reports success while SQLite stays 3.50.x" verification gap.

@teknium1 teknium1 closed this Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/install-update Installer, updater, packaging, wheels, doctor comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hermes update cannot fix WAL-reset risk on Ubuntu 26.04 (system SQLite stuck < 3.51.3) — venv rebuild on uv CPython 3.13 does

4 participants