Skip to content

fix+feat: batch v0.50.237 — 21 PRs (embedded terminal, JSON/diff viewers, MCP UI, cron tracking, workspace CRUD, archive upload, DeepSeek V4, NVIDIA NIM, fixes)#1243

Merged
nesquena-hermes merged 68 commits intomasterfrom
stage-batch-apr29
Apr 29, 2026
Merged

fix+feat: batch v0.50.237 — 21 PRs (embedded terminal, JSON/diff viewers, MCP UI, cron tracking, workspace CRUD, archive upload, DeepSeek V4, NVIDIA NIM, fixes)#1243
nesquena-hermes merged 68 commits intomasterfrom
stage-batch-apr29

Conversation

@nesquena-hermes
Copy link
Copy Markdown
Collaborator

Batch release v0.50.237 — 21 PRs

This PR merges 21 contributor PRs that passed full review, test suite (3007 passed, 0 failed), and Opus security/correctness review.


Added

Fixed


Absorb fixes (issues found during review, fixed in this PR)

  • XSS in MCP delete button — replaced onclick="...esc(s.name)..." with data-mcp-name + event delegation
  • Zip/tar-slip path traversal — is_relative_to() instead of startswith; actual byte tracking for zip-bomb limit
  • NVIDIA NIM endpoint missing — added to _OPENAI_COMPAT_ENDPOINTS and _SUPPORTED_PROVIDER_SETUPS
  • Terminal resize handle — element added to index.html; _terminalEls() now returns handle
  • Terminal env secrets — PTY shell env uses a safe allowlist, not os.environ.copy()
  • get_terminal() dead branch — second return term corrected to return None
  • Model dedup non-determinism — alphabetical provider_id sort before first-occurrence assignment
  • OAuth probe over-reach — pid regex validation + closed key_source allowlist
  • Cron double-run — guard rejects second "Run Now" when job is already tracked as running

Test results

3007 passed, 2 skipped, 3 xpassed, 1 warning, 8 subtests passed

All JS (node --check) and Python (py_compile) clean.


PRs on hold (feedback posted)

#1084, #1108, #1124, #1132, #1134, #1222, #1229, #1232 — detailed hold comments posted to each.

/cc @nesquena for independent review

fxd-jason and others added 30 commits April 29, 2026 04:30
When _loadOlderMessages prepends older messages, the viewport snaps
to the bottom instead of staying where the user was.

Two bugs compounding:
1. Wrong scrollable container. Code used `$("msgInner")` for scrollHeight
   and scrollTop, but #msgInner has no overflow-y — it is a flex column.
   The actual scrollable container is #messages (`.messages{overflow-y:auto}`).
   Setting msgInner.scrollTop was silently ignored.
2. renderMessages calls scrollToBottom at the end (ui.js:2552),
   which unconditionally scrolls #messages to the bottom and sets
   _scrollPinned=true. Since bug #1 made the scroll-restore a no-op,
   the page landed at the bottom every time.

Fix:
- Changed scroll restore target from `$("msgInner")` to `$("messages")`.
- Reset _scrollPinned = false after restoring the user position,
  so scrollToBottom does not re-fire on next tick.
- Add nvidia to _PROVIDER_DISPLAY, _PROVIDER_MODELS, and _PROVIDER_ALIASES
- Add nvidia to _PORTAL_PROVIDERS to preserve full model paths (e.g. qwen/qwen3-next-80b-a3b-instruct)
- Add NVIDIA_API_KEY to _PROVIDER_ENV_VAR for API key management
- Fixes 404 errors when using nvidia provider with models from multiple namespaces
Add loadDir('.') call in switchToProfile() Case B so the workspace file
tree panel reflects the new profile's workspace instead of showing stale
files from the previous profile.

Fix #1212: detect OAuth providers not in hardcoded set

Expand _OAUTH_PROVIDERS with copilot-acp and qwen-oauth.
Add fallback in get_providers() that checks hermes auth live status
for providers that have no API key and are not in the hardcoded set
(e.g. Anthropic connected via OAuth), so the Providers tab shows
them as configured.
Avoids unnecessary latency on the Settings page by restricting the
OAuth auth-status fallback to providers that are not in _PROVIDER_ENV_VAR.

Review feedback (PR #1221): the get_auth_status() call in the else branch
was firing for every unconfigured API-key provider (openai, anthropic, etc.),
adding a network round-trip per provider. Now it only runs for providers
that are not known API-key providers (custom/OAuth-capable providers).
When multiple providers expose the same bare model ID (e.g. two custom
providers both listing gpt-5.4), the model picker cannot distinguish
them — both rows appear active and clicking the other provider's copy
is a no-op.

Fix:
- Add _deduplicate_model_ids() post-process in api/config.py that
  detects duplicate bare model IDs across groups and prefixes
  collisions with @provider_id: so each entry is globally unique
- Update norm() regex in static/ui.js to strip @Provider: prefixes
  for fuzzy matching, so existing sessions with bare model IDs still
  restore correctly
- First occurrence stays bare for backward compatibility with sessions
  that already store the bare model name
- Update test_model_resolver to be dedup-aware

Closes #1228
Address reviewer questions:
- Document that first-occurrence ordering is not stable across
  config changes, but removing a provider causes re-dedup on next
  cache rebuild, so sessions still match the new bare entry
- Confirm @provider_id: format is consistent with existing
  _apply_provider_prefix() and resolved by resolve_model_provider()
  (splits on first ':')
- Use rsplit(':', 1) instead of split(':', 1) in resolve_model_provider()
  to handle provider_ids containing ':' (e.g. custom:my-key)
- Add note in _deduplicate_model_ids docstring about ordering instability
  across config changes (first occurrence wins is intentional)
- Add comment confirming N>2 provider dedup correctness
- Add tests for rsplit behavior with colon-containing provider_ids
- Mark test_sprint31 integration tests as xfail (pre-existing isolation
  issue)
Add deepseek-v4-flash and deepseek-v4-pro model entries to:
- api/config.py (_MODEL_LIST and _PROVIDER_MODELS)
- static/index.html (model dropdown)
- static/ui.js (static label map)

These are the latest DeepSeek models with 1M context window,
replacing the legacy deepseek-chat/deepseek-reasoner (deprecated 2026-07-24).
- Add zai (Z.AI / GLM / 智谱) to onboarding _SUPPORTED_PROVIDER_SETUPS
  with default model glm-5.1
- Add GLM models (glm-5.1, glm-5, glm-5-turbo, glm-4.x) to _MODEL_LIST
  for display in model dropdowns
- Update DeepSeek default_model from deepseek-chat-v3-0324 to deepseek-v4-flash
- Update DeepSeek default_base_url from /v1 to bare domain (API docs change)
- Remove deepseek-chat-v3-0324 (DeepSeek V3) and deepseek-reasoner (R1)
  from _MODEL_LIST, _PROVIDER_MODELS, static/index.html, and static/ui.js
- Keep only deepseek-v4-flash and deepseek-v4-pro
- These old model IDs are deprecated since 2026-07-24
Provider card improvements:
- Show model name tags when a provider card is expanded (panels.js)
- Add .provider-card-model-tag styling (style.css)

Custom providers in providers panel:
- Scan config.yaml custom_providers (e.g. glmcode, timicc) and list
  them as providers with their configured models (api/providers.py)
- Detect API key status from env var references (${ENV_VAR})
…dels

- Test custom_providers entries (glmcode, deepseek) appear in get_providers()
- Test env var reference detection (${VAR_NAME} pattern)
- Test bare API key, missing key, empty/malformed entries
- Assert DeepSeek V4 models present, V3 deprecated models removed
- Assert GLM model series in _PROVIDER_MODELS and onboarding setup
- Restore deepseek-chat-v3-0324 and deepseek-reasoner with '(legacy)' labels;
  these are deprecated 2026-07-24 but still live until then
- Fix zai (Z.AI/GLM) default_base_url: use /api/paas/v4 instead of /api/coding/paas/v4;
  the coding plan path is for the glmcode custom provider, not the general API
- Update test assertions to match
- queue: list-end (append to queue)
- interrupt: skip-forward (jump ahead)
- steer: compass (course correction)
…abled reason branching

- Drop btnCancel element and all JS show/hide call sites across
  boot.js, messages.js, sessions.js, ui.js (superseded by single
  primary action button)
- Remove .cancel-btn CSS rules including mobile media-query override
- Route updateSendBtn() title/aria-label through t() with English
  fallbacks; add composer_send/queue/interrupt/steer/stop keys to all
  7 locales (en, ru, es, de, zh, zh-Hant, ko)
- Branch disabled-state tooltip on reason: clarify lock, compression
  running, or idle-empty, each with its own i18n key
- Update test_sprint10 / test_sprint36 to reflect single-button model:
  assert btnSend present and id="btnCancel" absent; replace
  test_hides_cancel_button with test_clears_composer_status
The file tree already supported file rename (double-click), file delete
(button), and create file/folder.  This adds the missing directory
operations:

Backend:
- _handle_file_delete now supports directories when recursive=true
  (uses shutil.rmtree instead of blocking with an error)

Frontend:
- Right-click context menu on all file/directory items with Rename
  and Delete options (follows the project context menu pattern)
- Directory delete button (x) with confirmation dialog
- _inlineRenameFileItem() for renaming dirs via context menu prompt
- Expanded-dir cache is updated on rename/delete to stay consistent
- Context menu auto-positions within viewport bounds

i18n: delete_dir_confirm, rename_title, rename_prompt in all 7 locales

Closes #1104
… them going forward

Two artifacts from a contributor's local graphify (code-graph) tooling
slipped into PR #1233 (workspace drag-to-reorder):

  .graphify_cached.json    (3.5MB)
  .graphify_uncached.txt   (refs /home/fr33m1nd/hermes-webui-src/...)

Neither belongs in source control: the .json is an autogenerated cache
of node IDs for a graph visualisation tool, and the .txt is a
file-discovery index pointing at the contributor's local workspace
(/home/fr33m1nd/hermes-webui-src/) — paths that aren't valid for any
other developer.

The repo already ignores graphify-out/ but these two top-level dotfiles
weren't covered. Add explicit ignore entries and remove the tracked
copies.

No code change. CI remains green on 3.11/3.12/3.13.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Copy link
Copy Markdown
Owner

@nesquena nesquena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — batch v0.50.237 (21 PRs) ✅ (artifact cleanup pushed)

What this ships

Batch release of 21 contributor PRs (12 Added / 9 Fixed). Mix of feature work (embedded terminal, JSON tree viewer, inline diff viewer, MCP management, cron run tracking, archive upload, workspace CRUD/reorder, NVIDIA NIM, Z.AI/GLM, DeepSeek V4) and bug fixes (background unread dots, clarify drafts, mobile busy-input, sidecar repair hardening, scroll preservation, model picker dedup, model cache metadata, clarify-scope copy, workspace stale after profile switch).

Several of these PRs I'd already reviewed individually in earlier sessions (#1207, #1219, #1231, #1239, #1241, #1221) — that prior trace stands. The batch body documents 9 absorb-fixes the agent applied during integration; this review verifies the security-critical ones in their final form.

Traced against upstream hermes-agent

Pulled fresh nousresearch/hermes-agent tarball. The new agent-touching surfaces (terminal subprocess, archive extraction, MCP card management, NVIDIA NIM _OPENAI_COMPAT_ENDPOINTS, OAuth probe pid validation) are all WebUI-internal — no agent-side state mutation, no config.yaml shape changes that would round-trip through the CLI.

What I caught — accidentally-committed graphify artifacts

PR #1233 (workspace drag-to-reorder by @bergeouss) inadvertently committed two top-level dotfiles from the contributor's local graphify (code-graph) tooling:

.graphify_cached.json   3.5 MB  (3.4M-token autogenerated graph cache)
.graphify_uncached.txt          (file index referencing /home/fr33m1nd/hermes-webui-src/)

The repo already ignores graphify-out/ (.gitignore:46) but these top-level dotfiles weren't covered. The .txt file specifically references contributor fr33m1nd's local workspace path, so the cache will be invalid (and regenerated) for every other developer who pulls the branch.

Not a security issue, but pollution: 3.5 MB of noise that everyone now clones forever, plus a contributor's home-directory paths committed permanently into history.

What I pushed — c86545b

  • git rm on both .graphify_cached.json and .graphify_uncached.txt
  • Added explicit ignore entries to .gitignore so these can't recur

Other audit — security-critical absorbs verified in final form

The batch description claims nine absorb-fixes; I spot-checked the three that touch real attack surface:

1. Zip/tar-slip — api/upload.py:135-137,168-170

member_path = (dest_dir / member.filename).resolve()
if not member_path.is_relative_to(dest_dir.resolve()):
    raise ValueError(f'Zip-slip blocked: {member.filename}')

is_relative_to() on resolved Paths is the correct primitive (catches .. segments, symlinks, absolute paths inside the archive). Both zip and tar paths use the same check. Bomb-protection tracks actual extracted bytes inside the read loop (upload.py:152-158), not declared member.size/member.file_size — so a malformed declared-size header can't bypass the cap. ✅

2. Terminal env allowlist — api/terminal.py:156-161

_SAFE_ENV_KEYS = {
    "PATH", "HOME", "USER", "LOGNAME", "SHELL", "LANG", "LC_ALL",
    "LC_CTYPE", "LC_MESSAGES", "LANGUAGE", "TZ", "TMPDIR", "TEMP",
    "XDG_RUNTIME_DIR", "XDG_CONFIG_HOME", "XDG_DATA_HOME",
}
env = {k: v for k, v in os.environ.items() if k in _SAFE_ENV_KEYS}

Tight allowlist. No *_API_KEY, no *_TOKEN, no OAUTH_*, no OPENAI_*/ANTHROPIC_*/NOUS_* provider creds, no HERMES_* runtime config (everything except the explicit HERMES_WEBUI_TERMINAL=1 marker added below). PTY shell is interactive — leaking server creds into the user's local terminal would have been a real escalation; the allowlist prevents it. ✅

3. MCP delete-button XSS — static/panels.js:3286-3296

<button class="mcp-delete-btn" data-mcp-name="${esc(s.name)}" title="Delete">&times;</button>
...
// Delegate delete-button clicks — uses data-mcp-name to avoid inline onclick XSS
const name=btn.getAttribute('data-mcp-name');

s.name flows through esc() and lands in a data- attribute (not in onclick="..." where escaping would have to handle JS string context). The handler reads back via getAttribute — text content, not eval. Even a name like "); alert(1); (" is safe. ✅

End-to-end trace — abbreviated

The 21 sub-PRs were each individually reviewed by the agent (and several by me in prior sessions). Rather than re-trace 21 PRs end-to-end, I confirmed the integration:

  • No master deltas between batch HEAD and upstream master beyond the 21 listed PRs (per the file-list scan — every changed file maps to a documented PR in the body)
  • CI green on 3.11 / 3.12 / 3.13 (run 25092019036)
  • Local full suite: 2955 passed, 54 skipped, 3 xpassed, 0 failures (after my push)
  • Test count differs from PR body (3007 vs 2955): the body counts a different snapshot. Both show 0 failures, which is the load-bearing claim.
  • JS syntax: node --check was run by the agent on all changed JS files

Edge-case trace

Concern Status
Tar/zip path traversal is_relative_to() blocks it ✅
Tar/zip bomb (declared-size lie) Actual byte tracking ✅
Terminal leaks API keys Allowlist prevents ✅
Terminal leaks HERMES_* runtime state Allowlist prevents ✅
MCP name with HTML/JS in it esc() + data- attribute ✅
NVIDIA NIM endpoint missing → 404 silently Added to _OPENAI_COMPAT_ENDPOINTS
OAuth probe pid injection Regex validation + closed allowlist ✅
Cron double-run race Guard rejects second "Run Now" ✅
Model dedup non-determinism (provider order) Alphabetical sort before assignment ✅
get_terminal() dead-branch returning wrong sentinel Fixed to return None
Graphify artifacts re-committed Now ignored ✅ (after my push)

Tests

  • Local full suite: 2955 passed, 54 skipped, 3 xpassed, 0 failures (after my graphify cleanup push)
  • CI on PR HEAD: ✅ test (3.11), ✅ test (3.12), ✅ test (3.13) — re-runs after my push will refresh
  • New test files in this batch: test_1062_busy_input_modes.py, test_approval_card_layering.py, test_custom_providers_in_panel.py, test_embedded_workspace_terminal.py, test_issue1144_session_time_sync.py, test_issue1228_model_picker_duplicate_ids.py, test_issue483_inline_diff_viewer.py, test_issue484_json_tree_viewer.py, test_issue492_workspace_reorder.py, test_issue538_mcp_management.py, test_issue856_background_completion_unread.py, test_model_cache_metadata.py, test_model_scope_copy.py, test_parallel_session_switch.py, test_session_sidecar_repair.py — all pass

Minor observations (non-blocking)

  • The node regex check on .graphify_cached.json — even though it was committed accidentally, GitHub's diff renderer chokes on the 3.4M-token line, which is part of why the leak wasn't visually obvious during agent review. Worth a .gitattributes binary annotation if such files ever land again — but with the gitignore now in place, that's preventative.
  • The PR body's "3007 passed" is from the agent's pre-push snapshot; my run after the cleanup is 2955 (different test discovery surface — likely the agent ran with a few env-dependent extras enabled). Both are 0 failures.
  • The terminal _SAFE_ENV_KEYS allowlist intentionally omits LD_LIBRARY_PATH, LD_PRELOAD, DYLD_*, PYTHONPATH — correct call (these are local-attack vectors that shouldn't carry from the WebUI process to a fresh shell). One follow-up to consider: also explicitly strip SUDO_USER/SUDO_UID if present, though the allowlist's "deny by default" already excludes them.
  • The model dedup alphabetical sort is determinism by lexicographic order on provider_id; that's stable but may yield surprising "wins" (e.g. anthropic < openai < openrouter so anthropic always wins on collision). Not wrong — just a consequence of the chosen tiebreaker.

Recommendation

Approved (with artifact cleanup pushed). Large batch with a strong test surface (15 new test files, 0 failures local + CI). The three security-critical absorbs (zip/tar-slip, terminal env allowlist, MCP XSS) are correctly applied in the final form. The artifact leak I caught is non-functional pollution but worth fixing before merge — done in c86545b. Parked at approval — ready for the release agent's merge/tag pipeline.

@nesquena-hermes
Copy link
Copy Markdown
Collaborator Author

Thanks for the batch release summary, @nesquena! This is a substantial v0.50.237 release — 21 PRs covering embedded terminal, JSON/diff viewers, MCP UI, cron tracking, workspace CRUD, archive upload, new providers (DeepSeek V4, Z.AI/GLM, NVIDIA NIM), and a set of targeted bug fixes.

The absorb-fix list in the body is particularly useful — the XSS (MCP delete button), zip/tar-slip path traversal, and PTY env secret allowlist fixes are all high-value hardening items that make the batch safer to ship.

A few quick confirmation checks before merge:

Terminal endpoint security
The body mentions PTY shell env uses a safe allowlist (not os.environ.copy()). Can you confirm the allowlist is explicitly enumerated (e.g., PATH, HOME, TERM, LANG) rather than a blocklist of sensitive vars? Allowlist approach is correct — just confirming the implementation follows the right direction.

Zip-bomb byte tracking
The body mentions "actual byte tracking for zip-bomb limit" replacing an earlier approach. Is the limit enforced during extraction (streaming byte count) or only post-extraction (stat the extracted size)? Streaming enforcement is safer for large archives.

Cron double-run guard
The guard rejects a second "Run Now" when the job is already tracked as running — does this guard survive a server restart mid-run? (i.e., if the server restarts while a job is tracked as running, does the guard persist or reset, potentially leaving the job in a permanently-blocked state?)

Test count (3007 passed)
Impressive test coverage. The 3 xpassed entries — are those expected-failure tests that started passing (suggesting a behavior change), or known flaky infrastructure tests?

The overall shape looks solid and the Opus security review sign-off adds confidence. Happy to see this level of disciplined batching with embedded security hardening.

🤖 Automated triage via nesquena-hermes

@nesquena-hermes nesquena-hermes merged commit 72b4ff6 into master Apr 29, 2026
3 checks passed
@nesquena-hermes
Copy link
Copy Markdown
Collaborator Author

Merged in v0.50.237 via commit 72b4ff6. Thank you to all 21 contributors! 🎉

@nesquena-hermes nesquena-hermes deleted the stage-batch-apr29 branch April 29, 2026 05:31
JKJameson pushed a commit to JKJameson/hermes-webui that referenced this pull request Apr 29, 2026
…ers, MCP UI, cron tracking, workspace CRUD, archive upload, DeepSeek V4, NVIDIA NIM, security fixes) (nesquena#1243)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment