The salvaged proactive validation (#53307/#76896) rejects the
b'\x89PNG...' + zero-padding fixtures test_vision_tools.py used.
Replace all 11 sites with a real 1x1 PNG (+ ignored trailing padding
for size-sensitive tests), and pin _resize_image_for_vision passthrough
in the size-limit test so it exercises rejection, not resize recovery.
With both salvaged validators in place, the resolver-boundary verify()
(#53307) catches a truncated PNG before the full-decode gate (#76896)
runs. Either error message means the bytes stayed out of history.
Pillow is an optional dependency in this codebase (every other PIL use in
tools/vision_tools.py imports lazily and falls back). Both salvaged
validators now distinguish 'PIL missing' (pass through, header-only
sniff) from 'decode failed' (reject), so a Pillow-less install keeps
working instead of rejecting every PNG.
Follow-up to salvaged #53307 (@CannibalKush) and #76896 (@HaiyiMei).
MiniMax's Anthropic-compatible endpoint rejects an oversized native image
part with "media exceeds size limit: max 10485760 bytes (2013)" — no
occurrence of the word "image", so none of _IMAGE_TOO_LARGE_PATTERNS
matched. The 400 fell through to _REQUEST_VALIDATION_PATTERNS (the body
is type: invalid_request_error) and classified as format_error /
non-retryable.
That skipped the image-shrink recovery in conversation_loop, which is
gated on FailoverReason.image_too_large. Because the oversized part is
already baked into history as a tool_result image block, and the context
compressor rewrites text but not image data, every later turn re-sent the
same bytes and failed identically — the session stayed dead until the
user forked it.
Match on the "media" fragment, mirroring the existing "image exceeds"
entry so reworded vendor variants are caught too. A non-image media
rejection routed here is safe: the shrink pass finds no image parts,
returns False, and the caller surfaces the original error unchanged.
Fixes#76039
Second review round: honoring base64Encoded recursively let any nested
dict spoof {"base64Encoded": true, "data": "<secret>"} past the redactor
(Runtime.evaluate returns arbitrary by-value JSON), and two carriers were
missed entirely — Network.streamResourceContent returns unflagged binary
bufferedData, and Network.getRequestPostData's postData was not covered.
Replace the ambient field sets with per-method exact result-path specs:
_CDP_ALWAYS_BINARY_PATHS for declared-binary paths (screenshots, PDFs,
streamResourceContent, beginFrame screenshotData, the nested
CacheStorage.requestCachedResponse.response.body) and
_CDP_FLAGGED_BINARY_PATHS for paths whose carrier object's base64Encoded
sibling gates the exemption (Network/Fetch.getResponseBody body, IO.read
data, getRequestPostData postData). Path suffixes propagate only into the
matching subtree, so base64Encoded is type information solely on trusted
carrier objects — never ambient trust in nested JSON.
Architecture-review follow-up: the method-scoped binary_payload flag skipped
redaction for every string anywhere in the result of the two listed methods,
and the same corruption stayed reachable through Network.getResponseBody /
Fetch.getResponseBody / IO.read / Network.streamResourceContent. Make the
exemption field-scoped instead: an explicit schema exempts exactly
Page.captureScreenshot.result.data and Page.printToPDF.result.data (carriers
with no flag of their own), and any dict whose base64Encoded sibling is
exactly True exempts its body/data/bufferedData string (the protocol's own
discriminator — text bodies with base64Encoded: false stay redacted). Every
other string in every result keeps full secret redaction.
_redact_cdp_output applied redact_sensitive_text(force=True) to every string
in CDP results, including the base64 screenshot/PDF payload of
Page.captureScreenshot and Page.printToPDF. The Fernet pattern (gAAAA + base64
alphabet) matches arbitrary spans inside such payloads wherever gAAAA follows a +
or /, collapsing them to first6...last4: decoded PNGs came out corrupt (valid header,
CRC failures mid-IDAT, no IEND), the persisted full copies in tool_result_storage
were redacted too, and vision_analyze then embedded corrupt images that the provider
rejected with 400 invalid_image - killing resume sessions with a misleading provider
error. Skip redaction for the two binary-payload methods: the payload is binary,
not free text, so there is no secret to protect there. Every other method keeps
full redaction.
_runtime_model_config merges the agent's current identity onto the row's
existing model_config JSON. For model and provider it only SET the key
when the agent attribute was truthy, while base_url/api_mode/
reasoning_config/service_tier already deleted stale values when falsy.
When an agent rebuilt with an empty provider (inheriting the profile
default) was persisted, the previous provider/endpoint survived in
model_config while _persist_live_session_runtime updated the model
column separately. Resume then read the fresh model from the column but
the STALE provider from model_config, silently routing the resumed chat
to the wrong endpoint (e.g. a VeniceAI/empero route under a model that
should run on the profile default).
Apply the same delete-on-falsy rule to model and provider, mirroring the
or-None deletion the CLI path (_persist_model_switch_to_session) already
uses, so a stale session state can never survive into a resume override.
Existing desynced rows self-heal on the next live metadata persist.
Adds regression tests: merge drops stale provider/model when the agent
attribute is falsy, a truthy provider overwrites the stale value, resume
overrides fall back to the billing provider instead of the stale
endpoint, a real-DB round trip heals an already-desynced row, and a
first write (existing=None) reflects only the agent's current identity.
Treat the ChatGPT Codex invalid image-data 400 as an image rejection so Hermes strips image parts and retries text-only instead of aborting the session. Add coverage for the exact error wording.
Truncated or corrupt image bytes baked into immutable conversation history
get re-sent on every retry. Kimi/Moonshot reject them with HTTP 400
'prepare image failed ... failed to decode image: invalid or unsupported
image format', which was missing from _IMAGE_REJECTION_PHRASES, so the
turn exhausted retries and wedged the session instead of stripping the
images and recovering.
Adds the phrase to the recovery list plus a regression test mirroring the
exact Kimi error body. Complements PR #76896 (proactive full-decode
validation in vision_tools) with reactive recovery for already-poisoned
sessions. Fixes#76884.
The observed wire error — 'Downloaded response does not contain a valid JPG, PNG, WebP, or ICO image.' — has no match in _IMAGE_CORRUPT_PATTERNS, so it falls through to the non-retryable 400 handler and the session replays the same image parts into the same 400 until /new.
Adds the full observed sentence to the pattern list. Deliberately not the shorter prefixes: a bare 'downloaded response does not contain a valid' also matches non-image 400s, and a negative test now pins that a downloaded-response certificate 400 keeps falling through to the existing handler. Covered on both the 400 path and the message-only path.
Reported by ryuhaneul in #69078.
The image_corrupt recovery stripped images from canonical messages, not
just the retry payload — a transient provider rejection (xAI 'Invalid
PNG image.') permanently erased history, breaking the copy-on-write
contract (e762a5a473). Strip only the per-call api_messages copy (its
rows are shallow copies; the strip replaces content instead of mutating
the shared parts list) and pin history isolation with two regressions
(#69104 sweeper review).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The prior commit removed the too-blunt generic fallback (any
non-retryable 400 with image parts present -> strip and retry), but
nothing in the test suite would fail if someone brought it back --
every existing test either targets a recognized image_corrupt wording
or has no image content in the request at all.
Add test_unrelated_400_with_image_parts_does_not_strip_or_retry:
same image-bearing setup as the positive recovery test, but the 400 is
an unrelated unsupported-parameter rejection with no image wording.
Asserts exactly one provider call (no retry), the error surfaces to
the caller, and -- the regression guard -- the single call's messages
still carry the image_url part. A reintroduced generic fallback would
strip it before surfacing this unrelated error, which is exactly the
P1 that got this design reverted in the first place.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review (Sol xhigh) on the prior commit found a P1: the generic strip-
and-retry fallback ("any non-retryable 400 with image parts present
strips and retries") was too blunt. It couldn't tell an actual
image-corruption 400 apart from an unrelated one — bad tool schema,
unsupported parameter, billing, content policy — that merely happened
to carry image parts in the request. Any of those would silently erase
vision history and retry the still-invalid request, degrading sessions
that were never bricked in the first place. That's worse than the bug
it was meant to fix.
Revert the generic fallback (agent/conversation_loop.py). Keep only
the classifier-routed path: FailoverReason.image_corrupt +
_IMAGE_CORRUPT_PATTERNS, checked before _IMAGE_TOO_LARGE_PATTERNS
because shrinking corrupt bytes can't repair them. Corrupt-image
wordings still route to strip-and-retry; everything else falls through
to normal (non-retryable) handling as before. Add xAI's second wire
wording for the same corruption class ("base64 string of provided
image cannot be decoded", returned on unaligned truncation vs "Invalid
PNG image." on aligned truncation) and a compound-message test pinning
that image_corrupt wins when a body matches both pattern lists.
Drop TurnRetryState.stripped_images_this_turn. It's unnecessary now
that only one branch is left: the branch already only retries when
_strip_images_from_messages reports it removed something, and that
helper strips every image part from the request in one pass — so a
second corrupt-image hit on the retried (now text-only) request has
nothing left to strip and falls through on its own. No separate
one-shot flag needed.
Add a run_conversation integration test at the sequenced-provider
layer: corrupt 400 on attempt 1, strip, retry succeeds on attempt 2,
with explicit before/after assertions on the outgoing image_url part.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: paultaki <paultaki@users.noreply.github.com>
The permanent-brick class in #69078: xAI returns 'Invalid PNG image'
when a re-serialized image part in replayed history becomes
undecodable. The existing image-error patterns cover only Anthropic
'exceeds max dimension' wordings and 'model does not support images'
strings, so the classifier lands on a generic non-retryable 400 and
neither the shrink path nor the strip path fires. Every subsequent
turn (even bare text) fails identically because the poison stays in
history — the session is permanently wedged until deleted.
Two recovery layers, deliberately separate:
- Semantic split: new FailoverReason.image_corrupt with
_IMAGE_CORRUPT_PATTERNS ('invalid png image' / 'invalid jpeg image'),
checked BEFORE _IMAGE_TOO_LARGE_PATTERNS in both _classify_400 and
_classify_by_message. Corrupt bytes route to strip-and-retry, never
to the shrink path (shrinking corrupt bytes cannot help).
- Generic fallback: any non-retryable 400 whose outgoing messages
still contain image parts gets one strip-and-retry via the existing
_strip_images_from_messages helper, guarded by a new
stripped_images_this_turn one-shot flag on TurnRetryState. This
un-bricks the session for any current or future provider wording
without adding another pattern list to maintain.
Item 3 from the report (multimodal-part integrity across FTS
persistence + compaction handoff) is a separate investigation and
remains follow-up work.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: paultaki <paultaki@users.noreply.github.com>
Follow-up to @AIalliAI's #60750 commits: with python_environ_get_secret
downgraded to medium, the high-severity python_os_environ pattern still
fired on the same os.environ.get("...KEY") line, re-escalating the verdict
the downgrade intended to avoid. Exempt every .get() form (non-secret =
config read; secret-shaped = scored medium by the dedicated pattern), and
apply the same medium grade to the sibling os.getenv() secret pattern —
same shape, same rationale (#60709 point 2).
The original ^(?!\s*#) prefix only skipped full-line comments starting
with '#'. An inline comment like:
cfg = environ.get('HOME') # os.environ available
still triggered python_os_environ because the regex matched the code part
before the '#'.
Two complementary fixes:
1. Replace ^(?!\s*#) with ^[^#\n]* in the regex — this rejects any line
where a '#' comment marker appears anywhere before os.environ.
2. Add _compute_docstring_lines() — a state machine that pre-computes
lines inside triple-quoted strings (docstrings) and skips them during
pattern matching. Also handles single-line self-contained docstrings.
6 new regression tests covering: inline comments, multi-line docstrings,
single-line docstrings, full-line comments, and a verification that real
bare dict(os.environ) code still triggers. All 85 tests pass.
Five targeted fixes for #60709 (reported by @mvanhorn):
1. ruby_env_secret: scope ENV[] to case-sensitive Ruby constant
((?-i:ENV)) — no longer matches Python env[key] dict access.
2. python_environ_get_secret: downgrade critical→medium — reading
an API key via os.environ.get() is normal auth, not exfiltration.
3. python_os_environ: skip comment lines with ^(?!\s*#) — no longer
flags os.environ references in docstrings or code comments.
4. deception_hide: downgrade critical→high + negative lookahead for
UX guidance context (unless/except/until/confirm/diagnose/verify).
5. oversized_skill: downgrade high→low + raise cap 1MB→5MB — large
skills are legitimate; structural size is informational only.
All 80 existing tests pass. 6 new verification tests added for each fix.
Review-fold from the 3-angle simplify pass:
- sed -Ei / -iE / --in-place now match the shell-critical tier (the
bare '\s-i\b' token missed combined short flags and the GNU long
form); read-only sed stays unflagged. Regression tests added.
- agent_config_contract joins plugin_guard's CODE_EXEMPT_PATTERN_IDS:
content-contract prose in plugin code files (docstrings/comments)
is the same false-positive class the existing agent_config_mod
exemption suppresses. Doc/config files keep the full pattern set.
Efficiency reviewer: 1.24x full-scan cost (+3.4ms/file, install-time
only), worst-case adversarial line 55us — no ReDoS exposure.
Follow-up hardening on top of #92249's tiered scoring:
- Shell-critical tier now also catches tee, and cp/mv with the config
file in destination position (cp/mv reads and .bak backups excluded).
A single '>' redirect must be preceded by a word/quote character so
markdown blockquotes and '->' arrows no longer match.
- Prose tier catches mid-line imperatives behind directive markers
('you must modify...', 'please update...', 'make sure to append...'),
which previously bypassed the line-start anchor.
- Prose instructions aimed at AGENT config files score critical again:
project-skill quarantine acts only on 'dangerous', so high/caution
silently converted 'quarantined' into 'allowed' for exactly the
sentence shape persistence attacks use (concern raised in #88952).
Hermes/other-agent config prose stays high/caution (setup docs
legitimately instruct config.yaml edits).
- New content-contract tier ('AGENTS.md should contain ...') at
high/caution — the shape is shared by authoring guides and attacks.
- .claude/settings and .codex/config gain the same shell-critical tier.
Verified against a 595-skill corpus: 0 skills blocked by these tiers
(main blocked 44 legitimate ones), all mattpocock repro skills from
#92021 install, and 20/20 attack corpus lines keep their verdicts.
Enough1122's review on #92249 asked whether language write APIs
(Python open('w')/write_text/os.replace/shutil, Node fs.writeFileSync/
appendFile) are covered by the agent-config persistence tiers. They are
not: those tiers score shell redirection, sed -i, and imperative prose
only; language-API calls surface just the informational *_ref finding.
Static regexes cannot tie a dynamically-built path to the config-file
destination without executing the skill, so this is a documented scope
cut rather than missing coverage — runtime install gates remain the
backstop. Scoring behavior is unchanged, so SCANNER_VERSION stays at
skills-guard-v2 and cached verdicts remain valid.
Refs #92249
Under #92021 the scanner no longer emits critical agent_config_mod /
hermes_config_mod findings for bare mentions - the migration script's
legitimate references now score as informational _ref findings and the
verdict is "safe" with zero modification-intent findings. Tighten the
assertion accordingly (verdict must be safe, not merely non-dangerous)
and swap the known-false-positive set to the new _ref ids.
The skills-guard-v1 scanner flagged ANY mention of AGENTS.md / CLAUDE.md /
.cursorrules / .clinerules as critical/persistence. Any critical finding
forces a dangerous verdict, and community installs cannot be overridden
with --force — so legitimate meta-skills that merely DISCUSS agent config
files (authoring guides, setup docs, cross-references) were permanently
blocked. Three popular community skills were hit in the wild.
skills-guard-v2 scores the persistence category in three tiers:
- Mechanical persistence (shell redirection or sed -i targeting an agent
config file) stays critical -> dangerous. An unambiguous write path.
- Modification language in imperative position (verb at line/bullet start
within 80 chars of the filename) is high -> caution. Regexes cannot
separate "Edit AGENTS.md to inject instructions" from descriptive prose,
but imperative verbs are the shape real instructions take. Caution keeps
the install confirmable instead of irreversibly blocked.
- Bare references drop to low/informational for auditability without
driving the verdict.
The verb-proximity shape matches the existing convention in
tools/threat_patterns.py, and the tiering mirrors how allowed_tools_field
was already handled. The pattern id agent_config_mod is preserved so
plugin_guard.CODE_EXEMPT_PATTERN_IDS stays valid; hermes_config_mod /
other_agent_config get parallel _shell / _ref splits fixing the whole bug
class. SCANNER_VERSION bumps to v2 so cached v1 dangerous verdicts are
invalidated and re-scanned on next install attempt.
* fix(execute_code): limits line teaches spillover — big stdout is saved, not lost (follow-up to #97043)
* fix(execute_code): drop the editorializing tail from the limits line (maintainer review)
* refactor(execute_code): integrate kernel persistence into the core description (712 -> 654 tok/call, -8%)
* fix(execute_code): honest interpreter note — Hermes's own python is the common case; project venv only when VIRTUAL_ENV/CONDA_PREFIX is active
Drives the REAL session.resume -> _make_agent -> AIAgent path (eager_build)
through tui_gateway.server.handle_request against a real seeded state.db in
an isolated HERMES_HOME — no mocks. Three scenarios from the live report:
deleted provider falls back to default, renamed provider heals, and a legacy
canonical Bot Chat (no follow_profile_config marker) follows the profile's
current config.
A/B verified: all 3 FAIL on origin/main with the reported
"resume failed: Unknown provider '<name>'"; 3/3 pass with the salvaged
fixes + backfill.
Bot Chats created before the follow_profile_config marker existed carry no
contract in model_config, so they would stay pinned to a stale stored
provider until deleted — the exact shape of the live reports (#89497,
#94818). Mirror the plugin's own identity rule (the profile's session
titled exactly 'Bot Chat') as a legacy fallback in
_stored_session_runtime_overrides, matching the room-plumbing legacy
'Group:' title fallback.
Follow-up to the salvaged #90343 (@curator8888) and #96111 (@lorzl).
Bot-Mode canonical chats (the ONE forever DM per bot) and room plumbing
sessions are plugin-owned scratch conversations. They are now created with
an explicit follow_profile_config contract, persisted in the session row's
model_config, so session.resume rebuilds from the member profile's CURRENT
config instead of restoring the stored model/provider pin from an old row.
That stale pin is what left bot DMs stuck on a dead provider (e.g. 'out of
Nous credits' after the profile was switched to ollama-cloud) while the
same bot worked fine in rooms — the mirror image of the room-plumbing bug
(#89497 class). Normal 1:1 user chats keep the stored-runtime restore:
opening an older chat must show the model it actually used.
- tui_gateway/methods_session.py: accept follow_profile_config on session.create
- tui_gateway/server.py: persist the marker in the row; skip stored-runtime
overrides on resume when present
- apps/desktop/src/plugins/hermes-bots/plugin.js: send the contract from
createCanonicalChat and ensureGroupChatSession
- tests: backend override + row-persist coverage; desktop source-contract
coverage for both session kinds
Room member sessions in Bot Mode are per-member scratch conversations
inside a group chat. session.resume restored their stored model/provider
pin from the row's model_config, so a room bot stayed stuck on whatever
provider was pinned when the row was first written — even after the
profile was switched. Every room message then failed on the stale
provider (e.g. 'out of Nous credits' after switching a profile from
Nous to ollama-cloud) while the same bot worked fine in DMs.
Add an explicit room_plumbing contract:
- session.create accepts room_plumbing: true, persisted in model_config
- _stored_session_runtime_overrides() returns {} for marked rows, so a
room session always rebuilds from the member profile's CURRENT config
- hidden + 'Group:' title shape is kept as a legacy fallback for rows
created by older desktop builds that never sent the marker; hidden
non-room chats keep the stored-runtime restore
- Desktop Bot Mode sends room_plumbing: true when creating the hidden
per-member room sessions
Fixes#89497
A session row persists the provider identity a chat actually used. When that
provider is later renamed or removed (e.g. a custom_providers:/providers:
entry deleted, or a provider renamed oldone->newone), Desktop/TUI resume
restores the stale name into agent init and dies with:
agent init failed: Unknown provider '<name>'
while the CLI resumes the same session fine with the configured default.
- runtime_provider: add is_routable_provider() (full resolution chain:
built-in -> providers: -> custom_providers: -> models.dev)
- _stored_session_runtime_overrides: heal a non-routable provider via
canonical_custom_identity (base_url -> model -> configured provider),
drop to the configured default when unrecoverable, and clear the stale
base_url after healing so a dead endpoint cannot override the registry URL
- _start_agent_build: gate deferred-resume overrides on provider routability;
when the stored provider is gone, prefer the model the user picked for THIS
session, else the configured default
- tests: is_routable_provider cases, heal/fallback round-trips, gate checks
Refs #75128
Test-pollution class: runtime_provider is usually imported lazily (inside
switch_model's resolution path), so its first import in a pytest worker can
happen while a test has hermes_cli.config.load_config patched. The
module-level from-import then bound the MagicMock permanently — after the
patch exited, every later caller in the process silently read the dead
test's config. Live victim: MoA aggregator context-length resolution
(resolve_runtime_provider -> AuthError 'Unknown provider custom:example'),
making TestMoAContextLength::test_moa_custom_context_configures_compressor_threshold
fail whenever it shared a process with
TestLocalOllamaModelDiscovery::test_switch_model_on_current_ollama_custom_endpoint_keeps_base_url.
Fix: load_config / get_compatible_custom_providers / normalize_extra_headers
become late-bound delegates resolving hermes_cli.config attributes at call
time. Both patch targets (config.load_config and
runtime_provider.load_config) keep working. Regression tests pin the
late-binding property and fail if the delegates revert to from-imports
(sabotage-verified).
Live on both providers (verified 2026-08-28 against openrouter.ai/api/v1/models
and inference-api.nousresearch.com/v1/models) but absent from both curated
picker lists. Adds the entry directly below qwen3.8-max per newest-first
family ordering, an explicit 1M DEFAULT_CONTEXT_LENGTHS entry (new family
slug would otherwise fall through to the generic qwen 131072 catch-all —
same class as #69881), and regenerates model-catalog.json.
Scoped rollout: only the named providers touched. Pricing snapshot skipped
(both routes bill via official_models_api live pricing). Reasoning floor
already fires via the qwen3 prefix entry (180s, verified).
Check both resolved and unresolved path parts so a symlinked named
profile (e.g. profiles/dev -> /mnt/data/dev) is still detected.
Also use _ensure_cron_dir for output_dir in ensure_dirs() for consistency.
Simplify-code Phase 2 finding (medium severity).
Replace #96637's inline active_profile_homes() closure with #96508's
module-level _existing_profile_homes() filter (testable in isolation).
Widen _ensure_cron_dir from 3 to 12 mkdir sites across cron/ so every
directory creation fails closed for deleted named profiles, not just
the 3 originally protected. Add _is_named_profile_path() that checks
'profiles' in path parts (works for subdirs like cron/output/<job> and
scripts/ that the original parent.name heuristic couldn't reach).
Co-authored-by: misterdas <das7514@gmail.com>
Inimitable Mind