Skip to content

fix: type guard in mask_api_key, remove dead skill ref, explicit utf-8 encoding - #29608

Closed
annguyenNous wants to merge 1 commit into
NousResearch:mainfrom
annguyenNous:fix/mask-api-key-type-guard-encode-utf8
Closed

fix: type guard in mask_api_key, remove dead skill ref, explicit utf-8 encoding#29608
annguyenNous wants to merge 1 commit into
NousResearch:mainfrom
annguyenNous:fix/mask-api-key-type-guard-encode-utf8

Conversation

@annguyenNous

Copy link
Copy Markdown
Contributor

Summary

Fixes 3 classes of bugs:

1. Type guard in _mask_api_key_for_logs (run_agent.py)

The method accepts key: Any but calls len(key) without checking if key is a string. A non-string truthy value (e.g. int) would crash with TypeError: object of type 'int' has no len().

Fix: Add if not isinstance(key, str): return repr(key)[:12] after the callable/None checks.

2. Dead skill reference in kanban_swarm.py

skills=["avoid-ai-writing"] references a skill that was removed in v0.14.0, causing crash loops when synthesizer tries to load it.

Fix: Changed to skills=[].

3. Explicit encoding="utf-8" in security-sensitive .encode() calls

~100 .encode() calls across the codebase rely on the default UTF-8 encoding. While correct in Python 3, explicit encoding is best practice for security-sensitive code (HMAC, SHA256 hashing) to prevent cross-platform surprises.

Fix: Added encoding="utf-8" to:

  • hermes_cli/webhook.py: HMAC signature computation + payload encoding
  • hermes_cli/auth.py: Z.AI key hash (2 locations) + MiniMax PKCE challenge
  • gateway/run.py: API key fingerprint + session fingerprint

Test Plan

  • All 5 modified files pass py_compile
  • CI tests pass

…8 encoding

- Add isinstance(key, str) guard in _mask_api_key_for_logs() before
  calling len(). The type hint is Any — a non-string truthy value
  (e.g. int) would crash with TypeError on len(). Now returns
  repr(key)[:12] for non-string non-callable values.

- Remove hard-coded skills=['avoid-ai-writing'] in kanban_swarm.py.
  The skill was removed in v0.14.0 but the reference remained,
  causing crash loops. Changed to empty list.

- Add explicit encoding='utf-8' to all .encode() calls in
  security-sensitive code (HMAC, SHA256 hashing):
  * hermes_cli/webhook.py: HMAC signature + payload encoding
  * hermes_cli/auth.py: Z.AI key hash + MiniMax PKCE challenge
  * gateway/run.py: API key fingerprint + session fingerprint
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery labels May 21, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

The kanban_swarm.py dead skill reference fix (item 2) duplicates #29067 / #29485 (same one-line fix: remove avoid-ai-writing from skills=[]). The type guard and explicit UTF-8 encoding fixes are independent.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the defensive cleanup. The type-guard idea remains relevant on current main, but this needs a narrow revision before salvage.

Problems

  • The new repr(key)[:12] fallback would disclose a non-string credential value in a masking helper. The value reaches the debug-dump Authorization header through agent/agent_runtime_helpers.py:1457; return a fixed redacted marker instead.
  • The kanban_swarm.py hunk is superseded. Current main uses skills=["humanizer"] at hermes_cli/kanban_swarm.py:211, with regression coverage at tests/hermes_cli/test_kanban_cli_dispatch_passthrough.py:119; this was implemented in 69b74c15a324fcac460b5a143e5662036dae6387.
  • Please add a direct non-string regression test; tests/run_agent/test_run_agent.py:813 currently covers only None and string inputs.

Suggested changes

  • Preserve redaction for all unsupported key types and drop the stale kanban hunk.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
@teknium1

teknium1 commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Closing — the sites this PR touched are already explicit-encoding on current main (PLW1514 ruff gate, enabled and enforced in lint CI). The class can't regress. Thanks for the sweep work.

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

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants