fix: handle transient kanban SQLite disk I/O errors - #2
Closed
nuch1011 wants to merge 359 commits into
Closed
Conversation
… bundle_content_hash) PR NousResearch#6656 added rel_path + \x00 prefixing to ``bundle_content_hash`` so a filename swap between two files in a bundle changes the digest. But it only patched the in-memory side — ``content_hash`` in ``tools/skills_guard.py`` (the on-disk equivalent) still hashed file contents only. These two functions need to stay symmetric: ``check_for_skill_updates`` compares the disk hash of an installed skill against the bundle hash of the upstream copy. With the asymmetric fix, every clean install showed as drifted because the digests no longer matched (2 existing tests in ``test_skills_hub.py`` started failing as soon as the contributor's change landed). Apply the same ``rel_path + \x00 + content`` shape to the disk-side function. Both functions now produce the same digest for the same skill content laid out two ways. Documented the symmetry invariant in the docstring so a future change to either function knows to touch both. Also adds tests/tools/test_pr_6656_regressions.py with 10 regression tests covering all three fixes salvaged in PR NousResearch#6656: - uninstall_skill path traversal (4 cases: parent segments, absolute paths, symlink escape, legitimate skill) - bundle_content_hash filename swap detection (4 cases: in-memory swap, identity, disk-side swap, bundle↔disk symmetry) - list_pending lock contract (2 cases: source-grep contract, smoke) Also fixes AUTHOR_MAP entry for @aaronlab — their commit email (1115117931@qq.com) maps to "aaronagent" which isn't a real GitHub login, so changelog @mentions would 404.
…ousResearch#17656) `get_read_block_error` previously only denied reads inside `${HERMES_HOME}/skills/.hub`, which left `auth.json` (provider OAuth state + plaintext API keys) and `.anthropic_oauth.json` (Anthropic PKCE tokens) directly readable by the agent. A prompt-injection reaching `read_file` could exfiltrate active provider credentials in plaintext. Mode-0600 file permissions only protect against *other Unix users* — the agent runs as the file's owner, so `read_file` is unaffected. Extend the existing deny list with the three credential paths identified in NousResearch#17656 (`auth.json`, `auth.lock`, `.anthropic_oauth.json`). The check uses the same `Path.resolve()` pattern as `skills/.hub`, so symlink/path-traversal indirection is caught too. The agent doesn't need to read these directly — `auxiliary_client` and `credential_pool` consume them through process env / OAuth flows that bypass `read_file`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
read_file_tool resolves relative paths against TERMINAL_CWD (or the task's live terminal cwd), but the prior call passed the original unresolved string to get_read_block_error. That function's own resolve() is anchored at the Python process cwd, so when a task's TERMINAL_CWD pointed at HERMES_HOME and the agent issued read_file on the relative path "auth.json", the credential-store denylist was never reached and the file was read normally. Pass the already-resolved absolute path string at the file_tools call site, document the contract on get_read_block_error, and add a read_file_tool-level regression test that pins the relative-path case under TERMINAL_CWD == HERMES_HOME. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ts, root Extends @briandevans's PR NousResearch#17659 from {auth.json, auth.lock, .anthropic_oauth.json} to also cover: - HERMES_HOME/.env (provider API keys) - HERMES_HOME/webhook_subscriptions.json (per-route HMAC secrets) - HERMES_HOME/mcp-tokens/ (OAuth token directory; dir + everything inside) …AND iterates over both _hermes_home_path() AND _hermes_root_path() so profile-mode runs (HERMES_HOME = <root>/profiles/<name>) also block <root>/{auth.json, .env, mcp-tokens/, ...}. Same widening shape as the write-deny side already does (NousResearch#15981, NousResearch#14157). Explicitly NOT a security boundary. Per the personal-assistant trust model, the terminal tool runs as the same OS user and can `cat auth.json` directly. This read-deny exists as defense-in-depth: - Models that respect tool denials empirically tend to stop rather than reach for the shell. - The denial surfaces an audit trail when something tries to read credentials — easier to spot in logs than a generic `cat`. Docstring + error message both flag this as defense-in-depth so future contributors don't mistake it for a real security boundary and don't re-decline reports that propose the same fix shape. Absorbs the .env and mcp-tokens/ coverage from @tomqiaozc's parallel PR NousResearch#8055 (closed-as-duplicate, credited). Co-authored-by: Tom Qiao <zqiao@microsoft.com>
…Research#29156) GHSA-5qr3-c538-wm9j — half one of the bypass chain. ``_discover_dashboard_plugins`` opted into the untrusted ``./.hermes/ plugins/`` source via ``if os.environ.get("HERMES_ENABLE_PROJECT_ PLUGINS"):`` — which is True for any non-empty string. ``=0``, ``=false``, ``=no``, ``=off`` all return non-empty strings and so *enabled* the project source even though every operator (and the agent loader, ``hermes_cli/plugins.py`` line 815) reads those values as "disabled". An attacker who can land a manifest under the CWD's ``.hermes/plugins/`` directory — a malicious cloned repo, a worktree checked out from a forked PR, a CI runner workspace — was therefore guaranteed to get their manifest discovered the moment the user ran ``hermes dashboard`` from that directory, regardless of whether the user thought they had project plugins disabled. Switch to the shared ``utils.env_var_enabled`` helper used by the agent loader so the gate accepts the documented truthy set (``1`` / ``true`` / ``yes`` / ``on``, case-insensitive) and treats everything else — including ``0`` / ``false`` / ``no`` — as off. Half two (path-traversal + project-source ``api`` import) lands in the next commit. Together they break the RCE chain at two distinct choke points so a future regression in either one alone can't re-open the advisory.
…Research#29156) 35 new tests across 5 classes covering every layer of the GHSA-5qr3-c538-wm9j defence. Each class corresponds to one chokepoint so a regression in any single layer is caught by the named class: * ``TestProjectPluginsEnvGate`` (13 cases) — parametrised over both the documented truthy values (``1`` / ``true`` / ``yes`` / ``on`` + uppercase variants) and the previously-bypassing falsy strings (``0`` / ``false`` / ``no`` / ``off`` / ``""`` / ``False``). The falsy half is the direct env-bypass repro: pre-fix any non-empty string enabled the project source. * ``TestApiPathSanitizer`` (16 cases) — unit-level coverage of the new ``_safe_plugin_api_relpath`` helper. Absolute paths (``/etc/passwd``, ``/tmp/payload.py``, ``/usr/bin/python``), ``..``-traversal payloads (including nested ``subdir/../../..``), and non-string / empty / whitespace-only values must all return ``None``. Safe relative paths (``api.py``, ``backend/routes.py``) round-trip unchanged so legitimate plugins keep working. * ``TestDiscoveryScrubsApiField`` (3 cases) — end-to-end through ``_discover_dashboard_plugins`` with a real manifest on disk. Verifies that the cached plugin entry's ``_api_file`` is scrubbed *at discovery time* (``None`` + ``has_api: False``) so any downstream consumer can't be tricked into re-deriving the unsafe path from cache. * ``TestMountApiRoutesRefusesUntrusted`` (3 cases) — pokes synthetic plugin entries with each refusal vector directly into the cache and patches ``importlib.util.spec_from_file_location`` to assert it is *not* invoked for project-source / traversal payloads, and *is* invoked normally for bundled / user plugins. * ``TestEndToEndPocBlocked`` (1 case) — reproduces the original advisory PoC: operator sets ``HERMES_ENABLE_PROJECT_PLUGINS=0`` believing project plugins are off, attacker plants a manifest in CWD's ``.hermes/plugins/`` with ``api`` pointing at an absolute payload path. Asserts that the importer is never called against the payload path *and* that ``hermes_dashboard_plugin_evil`` is not in ``sys.modules`` after the mount routine runs. An autouse fixture busts ``_dashboard_plugins_cache`` before and after each test so the production cache (populated by the import-time ``_mount_plugin_api_routes()`` call) can't bleed in. All 12 pre-existing dashboard-plugin tests in ``test_web_server.py`` still pass unchanged.
…arch#29156) GHSA-5qr3-c538-wm9j — half two of the bypass chain. ``_mount_plugin_api_routes`` imports each dashboard plugin's manifest ``api`` field as a Python module via ``importlib.util.spec_from_file_location`` — arbitrary code execution by design. Two primitives in the surrounding code turned that "by design" RCE into a usable attack: 1. Absolute paths in the manifest swallow the plugin directory. ``Path('safe/dashboard') / '/tmp/evil.py'`` resolves to ``/tmp/evil.py``, so a single manifest line ``{"api": "/tmp/payload.py"}`` was enough to redirect the importer at any Python file on disk. 2. ``..`` traversal in the manifest climbs out of the dashboard directory. ``Path('plugins/safe/dashboard') / '../../../tmp/evil.py'`` lands in ``/tmp/evil.py`` after ``resolve()`` — the static-asset handler (``serve_plugin_asset``) already defends against this via ``is_relative_to``; the api-mount path didn't. Fix at three layers so a regression in any one can't re-open the advisory: * New ``_safe_plugin_api_relpath`` validator runs at *discovery* time and stores only sanitised relative paths on the plugin entry's ``_api_file`` field. Absolute paths, ``..`` traversal, empty / non-string values, and paths that ``resolve()`` outside the plugin's ``dashboard/`` directory are rejected with a warning naming the plugin. ``has_api`` follows the sanitised value so the dashboard frontend doesn't render a fake "Backend API" badge for plugins whose api was scrubbed. * ``_mount_plugin_api_routes`` re-validates the resolved path against the live filesystem just before the import — defence in depth in case ``_dir`` is tampered with post-cache or a future caller bypasses the discovery-time validator. * Project plugins (``source == "project"``) are refused outright for backend import. ``./.hermes/plugins/`` ships with the CWD, so any threat model that includes "user opens a malicious repo" treats it as attacker-controlled; project plugins can still extend the UI via static JS/CSS but their Python ``api`` is no longer auto-imported. Combined with the truthy env-gate fix from the previous commit, the original advisory chain now fails at two distinct choke points.
…ch#29156) The reference entry now documents the truthy set (``1`` / ``true`` / ``yes`` / ``on``) explicitly, matches the falsy half (``0`` / ``false`` / ``no`` / ``off`` / empty string) that the GHSA-5qr3-c538-wm9j fix re-aligned both the agent loader and the dashboard web server around, and points readers at the defence-in-depth rule that project plugins never have their Python ``api`` file auto-imported by the dashboard regardless of the env var.
The two ACP slash-command tests that exercise `provider:model` routing
(`test_set_session_model_accepts_provider_prefixed_choice` and
`test_model_switch_uses_requested_provider`) relied on the live
`hermes_cli.models._KNOWN_PROVIDER_NAMES` / `_PROVIDER_ALIASES` module
state to parse `anthropic:claude-sonnet-4-6` into
`("anthropic", "claude-sonnet-4-6")`. If any earlier test in the same
xdist worker registers a custom provider that shadows `anthropic` or
otherwise mutates those globals, the parser falls into the
`detect_provider_for_model` branch and resolves to `custom` instead.
Observed once in CI on run 26326728502 / job 77505732299 as
`AssertionError: assert 'custom' == 'anthropic'` — could not reproduce
locally under per-file isolation, so the failing in-file order was
specific to a particular xdist scheduling.
Monkeypatching `parse_model_input` + `detect_provider_for_model` for
both tests removes the global-catalog dependency, so the tests now only
exercise what they were written to verify (the `requested_provider ->
runtime -> AIAgent kwargs` plumbing).
The Kimi K2 branch added in the prior commit only emitted extra_body.thinking and dropped reasoning_effort entirely. KimiProfile (api.moonshot.ai/v1) sends both fields, and OpenCode Go proxies to the same Moonshot backend. Mirror that shape on the Go path so /reasoning effort actually reaches Kimi. - low/medium/high pass through verbatim - xhigh/max clamp to high (Moonshot's max supported value) - minimal / unknown effort → omit reasoning_effort, keep thinking on - disabled / no config → unchanged - DeepSeek branch unchanged
…ad of silently swallowing
…ousResearch#30854) PR infographics belong in PR descriptions, not committed to the repo. Removes the 13 archived directories under infographic/ and adds the path to .gitignore so future generations don't accidentally land in-tree. The fal.media URLs embedded in each PR's body remain the canonical artifact — those PR descriptions are the storage.
PR #41d2c758c ("Fix unsafe gateway media path delivery") tightened
`validate_media_delivery_path` so that artifacts emitted by the agent
must live inside `MEDIA_DELIVERY_SAFE_ROOTS` (Hermes-managed cache
dirs) or an operator-allowlisted root via `HERMES_MEDIA_ALLOW_DIRS`.
Two kanban-notifier tests put their PDFs and PNGs under pytest's
`tmp_path`, which is correctly rejected by the new validator. They
started failing on main as soon as that PR landed:
FAILED tests/hermes_cli/test_kanban_notify.py::test_notifier_uploads_artifacts_on_completion
FAILED tests/hermes_cli/test_kanban_notify.py::test_notifier_artifact_delivery_skips_missing_files
Symptom in logs: "Skipping unsafe local file path outside allowed
roots". The validator is doing exactly what it should — the tests were
relying on the looser pre-fix behaviour.
Fix: add `HERMES_MEDIA_ALLOW_DIRS=tmp_path` to the `kanban_home`
fixture so artifacts under `tmp_path` are recognised as safe. This is
the same allowlist mechanism the operator-facing env var documents.
A bare except in _load_gateway_runtime_config would silently return the unexpanded dict on any _expand_env_vars failure — masking the very bug this helper exists to fix. Drop it; let the caller see real errors.
1. Add INTERACTION intent bit (1<<26) to _send_identify, fixing approval button clicks not being received (INTERACTION_CREATE events were never dispatched by the gateway) 2. Include local cached path in video/file attachment descriptions so the LLM can reference files for re-sending to users 3. Add unit tests (TestIdentifyIntents, TestProcessAttachmentsPathExposure)
1. Handle op 7 (Server Reconnect): close WS to trigger reconnect loop while preserving session for Resume 2. Handle op 9 (Invalid Session): check d value to determine if session is resumable; clear session only when not resumable 3. Remove 4009 from session-clearing set (connection timeout is resumable) 4. Expand fatal close codes: 4001/4002/4010-4014 now stop reconnect immediately instead of retrying uselessly 5. Add unit tests
Add original_name parameter to _download_and_cache, preferring the attachment metadata filename over the CDN URL path basename. Previously files were cached with meaningless QQ CDN hash names (e.g. qqdownload_...oadftnv5), causing ugly filenames when sent back to users. Aligns with qqbot-agent-sdk's AttachmentDownloader.download_document.
_guess_ext_from_data: data[:5] == b"#!SILK" -> data[:6] (6-byte string) _looks_like_silk: data[:4] == b"#!SILK" -> data[:6] The previous slices were too short to ever match the 6-byte "#!SILK" literal, relying entirely on the "#!SILK_V3" (9-byte) and 0x02! (2-byte) fallback paths for SILK format detection.
X Premium+ also grants Grok OAuth access — the 'SuperGrok Subscription' wording suggested SuperGrok was the only entitlement path. Updated to 'SuperGrok / Premium+' across the picker label, setup wizard, auth flows, and docs so Premium+ subscribers know the row applies to them too.
…t encoding Two CI follow-ups to @benbarclay's NousResearch#30136 salvage: 1. scripts/run_tests_parallel.py — add 'docker' to _SKIP_PARTS so the new tests/docker/ harness doesn't run in the regular test (N) matrix. The harness builds the real Dockerfile in a session fixture, which can exceed pytest-timeout's 180s ceiling on ubuntu-latest where Docker IS available — it surfaced as 6 identical setup-timeout failures across slices 1–6 on the first CI run. The docker harness has its own dedicated runner via .github/actions/hermes-smoke-test (added in NousResearch#30136) plus the docker-lint workflow. Same treatment as tests/integration/ and tests/e2e/ — runs separately, not in the main shards. 2. hermes_cli/service_manager.py — pin encoding='utf-8' on the /proc/1/comm read_text call. Ruff PLW1514 enforcement rolled in between Ben's last push and the salvage; pure ruff-fix, no behavior change.
Follow-up to @benbarclay's NousResearch#30136 salvage. The pre-existing PID-1 contract tests in tests/tools/test_dockerfile_pid1_reaping.py (added with NousResearch#15012) hardcoded tini/dumb-init/catatonit as the only accepted inits, so they failed after NousResearch#30136 replaced tini with s6-overlay's /init. s6-overlay's PID 1 is s6-svscan, which reaps zombies non-blockingly on SIGCHLD — same contract the test exists to enforce. Two updates: * test_dockerfile_installs_an_init_for_zombie_reaping — accept 's6-overlay' as a known-installed marker (matches the s6-overlay install layer in Ben's Dockerfile). * test_dockerfile_entrypoint_routes_through_the_init — accept '/init' as a known-routed marker (s6-overlay's PID-1 binary lives at /init by convention). Both assertions still fire if a future Dockerfile rewrite drops the init entirely. Local: 7/7 pass.
Documents five approaches for adding tools beyond what the official image ships with: npx/uvx for npm/Python tools, ad-hoc apt installs that Hermes remembers, derived images for durability, sidecar containers for multi-service stacks, and upstreaming via issue/PR for broadly useful additions.
…ker-docs docs(docker): add 'Installing more tools in the container' section
Resolves the explicit "Known follow-up" left by commit 2f8ceea and the resulting CI failures in tests/docker/test_dashboard.py and tests/docker/test_s6_profile_gateway_integration.py. The product gap --------------- Every hermes runtime operation inside the container runs as the hermes user (UID 10000) via s6-setuidgid. But s6-supervise — spawned by s6-svscan running as PID 1 — creates each service's supervise/ and top-level event/ directories with mode 0700 owned by its effective UID (root). That left every s6-svc / s6-svstat / s6-svwait call from hermes hitting EACCES on the supervise/control FIFO and supervise/status — i.e. the entire S6ServiceManager lifecycle (register, start, stop, unregister) was inert in production. The 2f8ceea commit message called this out and deferred the fix. The audit changes that landed alongside it (defaulting docker_exec to -u hermes) made the integration tests reproduce the bug deterministically; the fix below resolves it. The fix: pre-create the supervise/ skeleton hermes-owned ---------------------------------------------------------- Reading s6's source (src/supervision/s6-supervise.c::trymkdir + control_init), the mkdir and mkfifo calls that build the supervise tree are EEXIST-safe: if the directory or FIFO is already present, s6-supervise reuses it and skips the chown/chmod fix-up that would normally make event/ 03730 root:root. So if we lay the skeleton down with hermes ownership before triggering s6-svscanctl -a, s6-supervise inherits our layout and never touches it. The death_tally / lock / status regular files written later by s6-supervise (still as root) land mode 0644 — world-readable — which is all s6-svstat needs. New module-level helper _seed_supervise_skeleton(svc_dir) in hermes_cli/service_manager.py lays down: svc_dir/event/ hermes:hermes 03730 svc_dir/supervise/ hermes:hermes 0755 svc_dir/supervise/event/ hermes:hermes 03730 svc_dir/supervise/control hermes:hermes 0660 (FIFO) svc_dir/log/event/ hermes:hermes 03730 (if log/ present) svc_dir/log/supervise/ hermes:hermes 0755 svc_dir/log/supervise/event/ hermes:hermes 03730 svc_dir/log/supervise/control hermes:hermes 0660 (FIFO) The log/ branch matters because the logger is a second s6-supervise instance — without it, unregister rmtree races on the logger's root-owned supervise dir even after the parent slot's supervise/ is hermes-owned. The helper is idempotent and swallows PermissionError on chown so it works equally well when called from root (cont-init.d) or hermes (runtime register). Wiring ------ 1. S6ServiceManager.register_profile_gateway calls _seed_supervise_skeleton(tmp_dir) just before publishing the slot via Path.replace. Runtime-registered profile gateways are set up by hermes. 2. container_boot._register_service does the same in the cont-init.d reconciliation path so boot-time-restored profile slots inherit the same layout. 3. New cont-init.d/015-supervise-perms script chowns the supervise/ and event/ trees for STATIC s6-rc services (dashboard, main-hermes). These are spawned by s6-rc before cont-init.d gets to run, so the EEXIST-trick doesn't apply; we chown the already-existing tree instead. s6-supervise keeps using the same files; it never re-asserts ownership on a running service. The script skips s6-overlay internal services (s6rc-*, s6-linux-*) so the supervision tree itself stays root-only. 015- slot is intentional: lex-sorts between 01-hermes-setup and 02-reconcile-profiles in the container's C-locale, so the chown finishes before the reconciler walks the scandir. Unregister teardown reordering ------------------------------ S6ServiceManager.unregister_profile_gateway now fires s6-svscanctl -an BEFORE rmtree (with a 200ms grace), so s6-svscan reaps the supervise child and releases its file handles on supervise/lock + supervise/status before we try to remove the directory. Previously rmtree raced s6-supervise on a set of files inside the supervise dir, and even with the parent supervise/ now hermes-owned, the contained files (death_tally, lock, status, written by root) could still be in use. Dashboard down-state redesign ----------------------------- The original PR NousResearch#30136 review fix wrote a 'down' marker file into /run/service/dashboard/ via cont-init.d/03-dashboard-toggle. That approach was broken in two ways: (a) /run/service/dashboard is a symlink to a TRANSIENT /run/s6-rc:s6-rc-init:<tmpdir>/ directory while s6-rc is mid-transaction; the touch landed in a soon-to-be-discarded tmp. (b) Even when written to the final /run/s6-rc/servicedirs/ location, the 'down' file is only consulted by s6-supervise at slot startup. s6-rc's user-bundle explicitly transitions 'dashboard' to 'up' on every boot, overriding any down marker. The right fix is the canonical s6 pattern: when HERMES_DASHBOARD is unset, the dashboard run script exits 0 and a companion finish script exits 125. Per s6-supervise(8), exit code 125 from the finish script is the 'permanent failure, do not restart' marker — equivalent to s6-svc -O. The slot reports as 'down' to s6-svstat, matching the reality that no dashboard process is running. When HERMES_DASHBOARD IS truthy, finish exits 0 and restart-on-crash semantics apply. 03-dashboard-toggle is removed (its function is now subsumed by the run/finish pair). Tests ----- Adds four unit tests for _seed_supervise_skeleton covering the produced layout, the log/ subservice case, the skip-when-no-log case, and idempotency. The live-container verification continues to live in tests/docker/test_s6_profile_gateway_integration.py and tests/docker/test_dashboard.py — both now pass against the rebuilt image. References ---------- * Skarnet skaware mailing list 2020-02-02 (Laurent Bercot + Guillermo Diaz Hartusch) on unprivileged s6 tool semantics: http://skarnet.org/lists/skaware/1424.html * just-containers/s6-overlay#130 — same EEXIST-preseed pattern, community-validated 2016 onward * https://skarnet.org/software/s6/servicedir.html — exit-code 125 semantics in finish scripts (cherry picked from commit c41f908)
…against historical-comment masquerade PR NousResearch#30136 CI: test_dockerfile_entrypoint_routes_through_the_init failed because the test hardcoded known_inits = ('tini', 'dumb-init', 'catatonit'). The PR replaced tini with s6-overlay's /init (which execs s6-svscan as PID 1) — same SIGCHLD-reaping contract, different name, so the substring scan against ENTRYPOINT missed it. Two-part fix: 1. Extend the accepted token list to include 's6-overlay', 's6-svscan', and '/init'. The contract these tests enforce is behavioural ('some PID-1 init reaps SIGCHLD'), so the names list is purely a recognition table and any reaper-capable family should qualify. 2. Harden test_dockerfile_installs_an_init_for_zombie_reaping (the sibling check) against comment-only matches. It was scanning the full Dockerfile text and only passed because the word 'tini' is still in a historical comment explaining why we used to use it. The next person to clean up that comment would have silently broken the test. New _instruction_text() helper joins only the parsed, non-comment Dockerfile instructions so stale comments can't satisfy the check. (cherry picked from commit ffc1bb6)
…ycle test After the supervise-perms fix lands, the s6 lifecycle actually works for the hermes user — hermes -p <profile> gateway start now genuinely brings the supervised gateway up rather than silently no-op'ing on EACCES. That exposes a latent bug in this test's assertion: it expected 'want up' to appear literally in s6-svstat output, but s6-svstat elides redundancies — when the slot is currently up AND s6 wants it up, the output is just 'up (pid N pgid N) X seconds'; the explicit 'want up' token only appears when current ≠ wanted (e.g. 'down (exitcode 1) … , want up' on a crash-loop). Add a small helper _svstat_wants_up() that reads the want-state correctly across both spellings: * 'up …' → wanted up (unless explicit 'want down') * 'down …, want up' → wanted up explicitly * 'down …' → wanted down Both stop and start assertions now use the helper. Also rewords the module docstring to acknowledge that the supervised process may succeed OR crash-loop depending on environment, but the want- state contract holds either way. (cherry picked from commit 02c933a)
…t image The new tests/docker/ suite (added by this PR) was being picked up by the sharded pytest matrix in tests.yml, where its session-scoped `built_image` fixture issued a 3-7min `docker build` under tests/docker/conftest.py's 180s pytest-timeout cap. Every test in the directory failed in fixture setup across all 6 shards. Fix the suite so it actually runs (not skips): 1. Wire the docker tests into docker-publish.yml's build-amd64 job, right after the existing smoke test. The image is already loaded into the local daemon as `nousresearch/hermes-agent:test`; set HERMES_TEST_IMAGE to that and the fixture's pre-built-image branch short-circuits the rebuild. 21 tests run in ~90s locally against a prebuilt image, no rebuild cost on top of the existing build step. 2. Exclude tests/docker/ from scripts/run_tests_parallel.py's default discovery so the sharded matrix in tests.yml stops trying to build the image. Explicit positional paths (`pytest tests/docker/` or `scripts/run_tests.sh tests/docker/`) still pick the suite up — the skip rule honors directory-level user intent, matching the existing per-file override pattern. The dedicated docker-tests step runs on every PR that touches docker code (the existing path filters on docker-publish.yml already cover `tests/docker/**` via `**/*.py`), so the suite gates real changes. (cherry picked from commit 4c48186)
…on Windows On Windows, the setuptools-generated hermes.exe launcher is a separate native process that spawns python.exe (the interpreter running the update code). os.getpid() returns the Python PID, but the launcher (which holds the file lock) is the parent. Without walking the parent chain, every 'hermes update' reports its own launcher as a concurrent instance - a false positive. This patch builds an exclusion set containing the Python process and its entire ancestor chain, so the running invocation never reports itself.
…ction Follow-up to @Strontvod's fix. Tests: - Five new tests in test_update_concurrent_quarantine.py cover the parent- chain exclusion: the .exe launcher is excluded, an unrelated sibling hermes.exe is still reported, multi-level ancestry is fully excluded, PID cycles in the parent chain don't hang, and a partially-stubbed psutil (no Process attribute) degrades gracefully instead of crashing. - New _fake_psutil_with_parent_chain helper builds a fuller stand-in (Process / NoSuchProcess / AccessDenied + process_iter) than the process_iter-only SimpleNamespace the older tests use. Hardening: - Broaden the except in the parent-walk to bare Exception. The original fix listed (NoSuchProcess, AccessDenied, ValueError), but those names are evaluated lazily during exception matching — if psutil is a partial stub without the attribute, the exception handler itself raises AttributeError that escapes. The function is documented as 'never raises' (the surrounding update flow depends on it), so the broader catch keeps the contract regardless of how the dependency is shaped. AUTHOR_MAP: - Map schepers.zander1@gmail.com -> Strontvod so the salvaged commit resolves to @Strontvod in the release notes. All 18 detect_concurrent + quarantine tests pass.
…-bf5898da feat(docker)!: s6-overlay container supervision (salvage of NousResearch#30136)
…-status-rule fix(tui): keep status rule one-line in skinny terminals
…ousResearch#27385) (NousResearch#31894) NousResearch#27385 reports that on macOS the browser sees the xAI 'authorization received' success page but Hermes still raises xai_callback_timeout. The loopback HTTP handler was silent — no log line on receipt, no log line on wait timeout — so triaging the gap between 'browser saw success' and 'CLI saw timeout' required either a code change or guesswork. Adds two INFO log lines: - Per callback hit (handler): path, has_code, has_state, has_error, truncated User-Agent. Booleans / fingerprints only — no actual code/state strings leak. - On wait timeout: report whether result.code or result.error was populated at deadline. Distinguishes three failure modes: 1. No hit log + timeout log w/ has_code=False has_error=False → xAI's IDP never reached the loopback (firewall, port-binding, IPv6/IPv4 mismatch, browser blocked private-network access). 2. Hit log w/ has_code=False has_error=False + timeout log → xAI hit the loopback without OAuth params (the bare-URL case the handler already 400s on). 3. Hit log w/ has_code=True + timeout log w/ has_code=False → result_lock contention or race; would indicate a real bug. 133/133 in tests/hermes_cli/test_auth_xai_oauth_provider.py, tests/hermes_cli/test_xai_oauth_pkce_token_exchange.py, and tests/run_agent/test_codex_xai_oauth_recovery.py.
…arch#31895) The locale switcher appeared broken because hardcoded markdown links (`](/docs/X)`) got double-prefixed by Docusaurus to `/docs/<locale>/docs/X` (404) in non-English locales, and the MDX hero `<a href>` on the index page escaped locale routing entirely. Changes: - Rewrite 922 `](/docs/X)` -> `](/X)` across 166 docs files (strip trailing .md too). Docusaurus prepends locale + baseUrl itself. - docs/index.md -> index.mdx; hero "Get Started" anchor -> Docusaurus <Link> so it stays inside the active locale. - Drop `ko` locale entirely from docusaurus.config.ts + delete i18n/ko/ (4 stale auto-translated kanban pages, <2% coverage, misleading). Verified `npm run build` succeeds for both en and zh-Hans; `build/zh-Hans/ index.html` has no /docs/zh-Hans/docs/... double-prefixed paths. PR2 will translate the 335 English docs into i18n/zh-Hans/.
The legacy runtime_calls[-1] == "anthropic" check in test_model_switch_uses_requested_provider failed in CI under specific test-shard scheduling with 'custom' == 'anthropic', across multiple unrelated PRs on 2026-05-25. The May 23 pin (commit 3127a41) monkeypatched parse_model_input + detect_provider_for_model to remove the dependency on live _KNOWN_PROVIDER_NAMES module state but the flake reappeared anyway — root cause still not reproducible locally even under stress runs. The other three assertions ("Provider: anthropic" in result, state.agent.provider == "anthropic", state.agent.base_url == "https://anthropic.example/v1") already prove fake_resolve_runtime_provider was called with requested="anthropic" for the model-switch step — the agent's provider and base_url come directly from that fake's return value. The tail-position check was redundant and the only assertion that flaked. Replaces runtime_calls[-1] == "anthropic" with "anthropic" in runtime_calls so the plumbing path is still covered without depending on call ordering.
….5-pro Follow-up on top of @jacevys' PR NousResearch#21437 cherry-pick: - _provider_model_ids() now also matches normalized == 'openai-api' for the live /v1/models fetch path, so users see the full catalog instead of just the curated list. - Add gpt-5.5-pro and gpt-5.3-codex to the curated list for parity with the existing 'openai' table (used as fallback when /v1/models fails). - Add scripts/release.py AUTHOR_MAP entry for jacevys so CI doesn't block the salvage PR.
…arch#25752) The bug: cron/scheduler.py:_resolve_cron_enabled_toolsets returns an LLM-supplied per-job enabled_toolsets verbatim. The disabled_toolsets passed to AIAgent was a hardcoded [cronjob, messaging, clarify] that ignored agent.disabled_toolsets from config.yaml. An LLM could call cronjob(action='add', enabled_toolsets=['terminal','file'], prompt='...') and the cron-spawned agent would receive terminal+file even when the operator had globally disabled them. Fix: new _resolve_cron_disabled_toolsets() helper that ALWAYS layers agent.disabled_toolsets on top of the cron baseline. AIAgent's disabled_toolsets takes precedence over enabled_toolsets, so this stops the bypass regardless of what the per-job override contains. This is the disabled-side fix. Three concurrent PRs (NousResearch#25842, NousResearch#25815, NousResearch#25780) proposed intersection-side variants on _resolve_cron_enabled_toolsets; this fix is more robust because it stops the leak at the precedence boundary AIAgent itself enforces, not at a layer above. Regression test reproduces the issue's PoC exactly: config.yaml has agent.disabled_toolsets=[terminal,file]; cron job has enabled_toolsets=[web,terminal,file]; assertion: AIAgent receives disabled_toolsets containing terminal AND file. Salvaged from PR NousResearch#25786 by @Schrotti77. Simplified the implementation: dropped a 23-line _normalize_toolset_list() helper (handled str/tuple/ set/garbage input shapes) in favor of the existing convention (agent_cfg.get('disabled_toolsets') or []) used elsewhere in the codebase. YAML always parses these as lists; the elaborate normalizer was theatre for shapes we never produce. Closes NousResearch#25752 Co-authored-by: teknium1 <127238744+teknium1@users.noreply.github.com>
Two defense-in-depth fixes on cron output path handling:
1. cron/jobs.py:update_job() rejects mutation of the immutable 'id' field
(raises ValueError). Dashboard PUT /api/cron/jobs/{id} converts this to
HTTP 400. Without this, an attacker who can reach the update endpoint
could rename a job's id to '../escape' and move its output directory
outside OUTPUT_DIR.
2. cron/jobs.py:_job_output_dir() validates job IDs before composing
paths: rejects '.', '..', '/', '\\', absolute paths, and Windows drive
prefixes. Used by save_job_output() and remove_job() so legacy unsafe
IDs (from before this guard) fail closed rather than half-applying a
shutil.rmtree or output write outside the sandbox.
Tests:
- update_job rejects {'id': '../escape'} without renaming
- remove_job(legacy '../escape' id) raises ValueError without deleting
files outside OUTPUT_DIR or removing the job from the store
- save_job_output rejects '..', './escape', 'nested/escape',
absolute paths
- dashboard PUT /api/cron/jobs/{id} with {'id': '../escape'} returns
400, job list unchanged
Salvaged from PR NousResearch#29826 by @zapabob. Simplified implementation:
- Dropped a 23-line _validate_job_output_id() helper using Path.parts
semantics. The inline check (path separators + dot-components +
is_absolute) is shorter and behaviorally identical.
- Dropped the secondary OUTPUT_DIR.resolve()/relative_to() check —
redundant once we reject any path separator at the input boundary.
- Dropped the _docs/2026-05-21_cron-output-path-hardening_codex.md
planning artifact (we don't check planning docs into the repo).
Co-authored-by: teknium1 <127238744+teknium1@users.noreply.github.com>
Co-authored-by: sgtworkman <178342791+sgtworkman@users.noreply.github.com>
Extend PR NousResearch#31716 to plugin setup paths that were also using bare getpass.getpass(): hindsight (4 sites), honcho, simplex, line. Same mechanical swap onto hermes_cli.secret_prompt.masked_secret_prompt.
Owner
Author
|
Closing in favor of upstream PR NousResearch#31973. The fork-local PR compared against a stale fork main and showed unrelated files/conflicts. |
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
20 participants
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.
Summary
disk I/O erroras proof that WAL is unsupported.Test Plan
git diff --check/usr/local/lib/hermes-agent/venv/bin/python -m pytest tests/test_hermes_state_wal_fallback.py tests/hermes_cli/test_kanban_core_functionality.py -q -o 'addopts='\n- Added-line no-secrets scan overgit diff\n\nKanban:t_1719c3ef