fix(tui,gateway): preserve explicit empty tool allowlists and fix failing check regressions by badMade · Pull Request #665 · badMade/hermes-agent

badMade · 2026-05-31T22:31:33Z

Motivation

Prevent a fail-open where an explicit empty TUI toolset allowlist ([]) was converted to None and interpreted as “all tools enabled”, re-exposing sensitive capabilities when operators explicitly disabled all tools.
Address additional failing CI checks uncovered on this branch so the PR remains green and safe to merge.

Description

Preserve an explicit empty allowlist by returning the resolved enabled list directly from _load_enabled_toolsets() instead of enabled or None so [] remains [] while None still signals “no filtering / all tools”.
Preserve an agent parent’s explicit empty enabled_toolsets in _background_agent_kwargs() by using the parent's enabled_toolsets when it is not None instead of falling back on truthiness.
Update TUI endpoints (tools.list, toolsets.list, tools.show) to distinguish None (all-tools sentinel) from [] (explicit no-tools) when computing what to display/return, and to compute enabled as set(enabled_toolsets or []) while using enabled_toolsets is None to mean “show everything enabled”.
Add regression tests covering the previously-failing TUI cases in tests/test_tui_gateway_server.py.
Fix failing gateway regressions surfaced by checks:
- restore Slack team_id initialization in GatewayRunner._is_user_authorized to avoid NameError and preserve workspace-scoped auth checks;
- reject unresolved placeholder webhook secrets (e.g. ${WEBHOOK_SECRET}) in webhook signature validation;
- in QQ C2C handling, skip attachment processing for unauthorized users while still forwarding text-only events.

Testing

python -m ruff check tui_gateway/server.py tests/test_tui_gateway_server.py gateway/run.py gateway/platforms/webhook.py gateway/platforms/qqbot/adapter.py
pytest -q tests/test_tui_gateway_server.py
Targeted gateway regressions:
- tests/gateway/test_unauthorized_dm_behavior.py::test_unauthorized_dm_pairs_by_default
- tests/gateway/test_webhook_adapter.py::TestValidateSignature::test_validate_placeholder_secret_rejects_literal_hmac
- tests/gateway/test_qqbot.py::TestVoiceAttachmentSSRFProtection::test_unauthorized_c2c_skips_attachment_processing
- additional affected gateway auth tests from the failing CI set

Codex Task

gemini-code-assist

Code Review

This pull request ensures that empty toolset allowlists (i.e., []) are preserved instead of being coerced to None or falling back to default toolsets, and adds corresponding unit tests. However, the implementation in _background_agent_kwargs uses getattr(agent, "enabled_toolsets", None) is not None to check for the attribute, which introduces a bug when enabled_toolsets is explicitly set to None (representing all tools enabled). In this case, it incorrectly falls back to _load_enabled_toolsets(). Using hasattr(agent, "enabled_toolsets") is recommended to properly handle this scenario.

gemini-code-assist · 2026-05-31T22:32:58Z

+        "enabled_toolsets": (
+            agent.enabled_toolsets
+            if getattr(agent, "enabled_toolsets", None) is not None
+            else _load_enabled_toolsets()
+        ),


Using getattr(agent, "enabled_toolsets", None) is not None to check if the parent agent has an explicit toolset allowlist introduces a bug when the parent agent has enabled_toolsets = None (which represents "all tools enabled").

In this case, getattr(agent, "enabled_toolsets", None) returns None, causing the condition to evaluate to False and falling back to _load_enabled_toolsets(). If _load_enabled_toolsets() returns a restricted list (e.g., ["web"]), the background agent will be restricted to ["web"] even though the parent agent had all tools enabled (None).

Using hasattr(agent, "enabled_toolsets") instead correctly preserves None while still falling back to _load_enabled_toolsets() if the attribute is missing entirely.

Suggested change

"enabled_toolsets": (

agent.enabled_toolsets

if getattr(agent, "enabled_toolsets", None) is not None

else _load_enabled_toolsets()

),

"enabled_toolsets": (

agent.enabled_toolsets

if hasattr(agent, "enabled_toolsets")

else _load_enabled_toolsets()

),

github-actions · 2026-05-31T22:34:09Z

🔎 Lint report: `badmade/fix-tui-tool-allowlist-vulnerability` vs `origin/main`

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 8259 on HEAD, 8257 on base (🆕 +2)

🆕 New issues (4):

Rule	Count
`invalid-argument-type`	3
`unsupported-operator`	1

First entries

gateway/platforms/qqbot/adapter.py:1211: [invalid-argument-type] invalid-argument-type: Argument to function `QQAdapter._merge_quote_into` is incorrect: Expected `str`, found `str | Unknown | (list[Unknown] & ~AlwaysFalsy)`
gateway/platforms/qqbot/adapter.py:1197: [unsupported-operator] unsupported-operator: Operator `+` is not supported between objects of type `str | Unknown` and `(Any & ~AlwaysFalsy) | (list[Unknown] & ~AlwaysFalsy) | (str & ~AlwaysFalsy)`
gateway/platforms/qqbot/adapter.py:1223: [invalid-argument-type] invalid-argument-type: Argument to function `QQAdapter._detect_message_type` is incorrect: Expected `list[Unknown]`, found `Any | list[Unknown] | str`
gateway/platforms/qqbot/adapter.py:1227: [invalid-argument-type] invalid-argument-type: Argument is incorrect: Expected `list[str]`, found `Any | list[Unknown] | str`

✅ Fixed issues (1):

Rule	Count
`unresolved-reference`	1

First entries

gateway/run.py:5544: [unresolved-reference] unresolved-reference: Name `team_id` used when not defined

Unchanged: 4357 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

Copilot

Pull request overview

This PR fixes a security-relevant “fail-open” behavior in the TUI toolset allowlist handling by preserving an explicitly empty allowlist ([]) end-to-end, rather than collapsing it into None (which is treated as “all toolsets enabled”).

Changes:

Preserve explicit empty toolset allowlists by returning the resolved enabled list directly from _load_enabled_toolsets() (so [] stays [], while None remains the “all toolsets” sentinel).
Preserve an agent parent’s explicit empty enabled_toolsets when constructing background agent kwargs (avoid truthiness-based fallback).
Update TUI listing endpoints to correctly distinguish None (all enabled) vs [] (none enabled), and add regression tests for the previously failing cases.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
`tui_gateway/server.py`	Fixes allowlist propagation and endpoint display logic so `[]` does not become “all enabled”.
`tests/test_tui_gateway_server.py`	Adds regression tests ensuring empty allowlists are preserved through config load, agent creation, and background agent kwargs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

github-actions · 2026-05-31T22:48:56Z

Auto-merge: review received — action required

Review activity detected (1 inline comment(s) by non-author(s); 2 comment(s)/review(s) from review bot(s): gemini-code-assist[bot], copilot-pull-request-reviewer[bot]).

To confirm you have reviewed and accepted the feedback, please add the reviewed label to this PR. Auto-merge will then proceed.

View workflow run

badMade · 2026-05-31T23:20:40Z

@claude code review

badMade · 2026-05-31T23:20:43Z

@jules code review

badMade · 2026-06-01T00:11:13Z

@copilot, fix all failing checks.

github-actions · 2026-06-01T01:23:48Z

Auto-merge: checks failing

The following checks did not pass:

test (failure)

Please fix the failing checks before this PR can be merged.

View workflow run

Copilot · 2026-06-01T01:45:02Z

@copilot, fix all failing checks.

Addressed the failing checks in commit 8bfa5c2. I fixed the gateway auth team_id regression, rejected unresolved placeholder webhook secrets during signature validation, and updated QQ C2C handling to skip attachment processing for unauthorized users while still forwarding text-only events.

fix(tui): preserve empty tool allowlists

501050d

Copilot AI review requested due to automatic review settings May 31, 2026 22:31

badMade added aardvark codex labels May 31, 2026 — with ChatGPT Codex Connector

Copilot started reviewing on behalf of badMade May 31, 2026 22:31 View session

gemini-code-assist Bot reviewed May 31, 2026

View reviewed changes

Copilot AI reviewed May 31, 2026

View reviewed changes

Merge branch 'main' into badmade/fix-tui-tool-allowlist-vulnerability

c7a2609

badMade added the reviewed label Jun 1, 2026

Copilot started work on behalf of badMade June 1, 2026 01:35 View session

fix(gateway): resolve failing auth and webhook regressions

8bfa5c2

Copilot AI changed the title ~~fix(tui): preserve explicit empty tool allowlists (avoid fail-open)~~ fix(tui,gateway): preserve explicit empty tool allowlists and fix failing check regressions Jun 1, 2026

Copilot finished work on behalf of badMade June 1, 2026 01:45

Merge branch 'main' into badmade/fix-tui-tool-allowlist-vulnerability

75aedfa

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(tui,gateway): preserve explicit empty tool allowlists and fix failing check regressions#665

fix(tui,gateway): preserve explicit empty tool allowlists and fix failing check regressions#665
badMade wants to merge 4 commits into
mainfrom
badmade/fix-tui-tool-allowlist-vulnerability

badMade commented May 31, 2026 •

edited by Copilot AI

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot May 31, 2026

Uh oh!

github-actions Bot commented May 31, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

github-actions Bot commented May 31, 2026 •

edited

Loading

Uh oh!

badMade commented May 31, 2026

Uh oh!

badMade commented May 31, 2026

Uh oh!

badMade commented Jun 1, 2026

Uh oh!

github-actions Bot commented Jun 1, 2026 •

edited

Loading

Uh oh!

Copilot AI commented Jun 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

badMade commented May 31, 2026 • edited by Copilot AI Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Description

Testing

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 31, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔎 Lint report: badmade/fix-tui-tool-allowlist-vulnerability vs origin/main

ruff

ty (type checker)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

github-actions Bot commented May 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Auto-merge: review received — action required

Uh oh!

badMade commented May 31, 2026

Uh oh!

badMade commented May 31, 2026

Uh oh!

badMade commented Jun 1, 2026

Uh oh!

github-actions Bot commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Auto-merge: checks failing

Uh oh!

Copilot AI commented Jun 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

badMade commented May 31, 2026 •

edited by Copilot AI

Loading

github-actions Bot commented May 31, 2026 •

edited

Loading

🔎 Lint report: `badmade/fix-tui-tool-allowlist-vulnerability` vs `origin/main`

github-actions Bot commented May 31, 2026 •

edited

Loading

github-actions Bot commented Jun 1, 2026 •

edited

Loading