Skip to content

fix(security): PKCE verifier leak, OAuth refresh Content-Type, tool_choice mcp_ prefix#1775

Merged
teknium1 merged 2 commits into
mainfrom
hermes/hermes-8058968e
Mar 17, 2026
Merged

fix(security): PKCE verifier leak, OAuth refresh Content-Type, tool_choice mcp_ prefix#1775
teknium1 merged 2 commits into
mainfrom
hermes/hermes-8058968e

Conversation

@teknium1

@teknium1 teknium1 commented Mar 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Salvaged from PR #1757 by @0xbyt4. Cherry-picked cleanly (7 commits behind).

Three security/correctness bugs in agent/anthropic_adapter.py:

1. PKCE code_verifier leaked via OAuth state parameter

run_hermes_oauth_login() set "state": verifier, exposing the PKCE secret in the authorization URL (browser history, proxy logs, Referer headers). Now uses a separate secrets.token_urlsafe(16) value.

2. refresh_hermes_oauth_token used wrong Content-Type

Sent application/json but RFC 6749 requires application/x-www-form-urlencoded for token endpoints. The other refresh function (_refresh_oauth_token) already used the correct format. Fixed to use urllib.parse.urlencode() + correct Content-Type.

3. tool_choice name not mcp_-prefixed for OAuth

When is_oauth=True, all tool names get mcp_ prefix but tool_choice did not, causing Anthropic API rejection (name mismatch). Now prefixes tool_choice to match.

Tests

3 new regression tests. All pass. No regressions (delegate test failures are pre-existing from #1778).

Closes #1757

Wrap json.loads() in load_transcript() with try/except JSONDecodeError
so that partial JSONL lines (from mid-write crashes like OOM/SIGKILL)
are skipped with a warning instead of crashing the entire transcript
load. The rest of the history loads fine.

Adds a logger.warning with the session ID and truncated corrupt line
content for debugging visibility.

Salvaged from PR #1193 by alireza78a.
Closes #1193
Rework _get_provider() to separate explicit config from auto-detect.
When stt.provider is explicitly set in config.yaml, that choice is
authoritative — no silent cross-provider fallback based on which env
vars happen to be set. When no provider is configured, auto-detect
still tries: local > groq > openai.

This fixes the reported scenario where provider: local + a placeholder
OPENAI_API_KEY caused the system to silently select OpenAI and fail
with a 401.

Closes #1774
@teknium1 teknium1 force-pushed the hermes/hermes-8058968e branch from 16bba2e to 9543f8b Compare March 17, 2026 16:51
@teknium1 teknium1 changed the title fix(stt): reject placeholder API keys in STT provider selection fix(stt): respect explicit provider config instead of env-var fallback Mar 17, 2026
@teknium1 teknium1 merged commit 9a1e971 into main Mar 17, 2026
1 check passed
@teknium1 teknium1 changed the title fix(stt): respect explicit provider config instead of env-var fallback fix(security): PKCE verifier leak, OAuth refresh Content-Type, tool_choice mcp_ prefix Mar 17, 2026
angelburgosrosado pushed a commit to angelburgosrosado/hermes-agent that referenced this pull request Apr 27, 2026
NousResearch#1775)

* fix(session): skip corrupt lines in load_transcript instead of crashing

Wrap json.loads() in load_transcript() with try/except JSONDecodeError
so that partial JSONL lines (from mid-write crashes like OOM/SIGKILL)
are skipped with a warning instead of crashing the entire transcript
load. The rest of the history loads fine.

Adds a logger.warning with the session ID and truncated corrupt line
content for debugging visibility.

Salvaged from PR NousResearch#1193 by alireza78a.
Closes NousResearch#1193

* fix(stt): respect explicit provider config instead of env-var fallback

Rework _get_provider() to separate explicit config from auto-detect.
When stt.provider is explicitly set in config.yaml, that choice is
authoritative — no silent cross-provider fallback based on which env
vars happen to be set. When no provider is configured, auto-detect
still tries: local > groq > openai.

This fixes the reported scenario where provider: local + a placeholder
OPENAI_API_KEY caused the system to silently select OpenAI and fail
with a 401.

Closes NousResearch#1774
02356abc pushed a commit to 02356abc/hermes-agent that referenced this pull request May 14, 2026
NousResearch#1775)

* fix(session): skip corrupt lines in load_transcript instead of crashing

Wrap json.loads() in load_transcript() with try/except JSONDecodeError
so that partial JSONL lines (from mid-write crashes like OOM/SIGKILL)
are skipped with a warning instead of crashing the entire transcript
load. The rest of the history loads fine.

Adds a logger.warning with the session ID and truncated corrupt line
content for debugging visibility.

Salvaged from PR NousResearch#1193 by alireza78a.
Closes NousResearch#1193

* fix(stt): respect explicit provider config instead of env-var fallback

Rework _get_provider() to separate explicit config from auto-detect.
When stt.provider is explicitly set in config.yaml, that choice is
authoritative — no silent cross-provider fallback based on which env
vars happen to be set. When no provider is configured, auto-detect
still tries: local > groq > openai.

This fixes the reported scenario where provider: local + a placeholder
OPENAI_API_KEY caused the system to silently select OpenAI and fail
with a 401.

Closes NousResearch#1774
olympus-terminal pushed a commit to olympus-terminal/hermes-agent that referenced this pull request May 16, 2026
NousResearch#1775)

* fix(session): skip corrupt lines in load_transcript instead of crashing

Wrap json.loads() in load_transcript() with try/except JSONDecodeError
so that partial JSONL lines (from mid-write crashes like OOM/SIGKILL)
are skipped with a warning instead of crashing the entire transcript
load. The rest of the history loads fine.

Adds a logger.warning with the session ID and truncated corrupt line
content for debugging visibility.

Salvaged from PR NousResearch#1193 by alireza78a.
Closes NousResearch#1193

* fix(stt): respect explicit provider config instead of env-var fallback

Rework _get_provider() to separate explicit config from auto-detect.
When stt.provider is explicitly set in config.yaml, that choice is
authoritative — no silent cross-provider fallback based on which env
vars happen to be set. When no provider is configured, auto-detect
still tries: local > groq > openai.

This fixes the reported scenario where provider: local + a placeholder
OPENAI_API_KEY caused the system to silently select OpenAI and fail
with a 401.

Closes NousResearch#1774
gweeteve pushed a commit to gweeteve/hermes-agent that referenced this pull request Jun 2, 2026
NousResearch#1775)

* fix(session): skip corrupt lines in load_transcript instead of crashing

Wrap json.loads() in load_transcript() with try/except JSONDecodeError
so that partial JSONL lines (from mid-write crashes like OOM/SIGKILL)
are skipped with a warning instead of crashing the entire transcript
load. The rest of the history loads fine.

Adds a logger.warning with the session ID and truncated corrupt line
content for debugging visibility.

Salvaged from PR NousResearch#1193 by alireza78a.
Closes NousResearch#1193

* fix(stt): respect explicit provider config instead of env-var fallback

Rework _get_provider() to separate explicit config from auto-detect.
When stt.provider is explicitly set in config.yaml, that choice is
authoritative — no silent cross-provider fallback based on which env
vars happen to be set. When no provider is configured, auto-detect
still tries: local > groq > openai.

This fixes the reported scenario where provider: local + a placeholder
OPENAI_API_KEY caused the system to silently select OpenAI and fail
with a 401.

Closes NousResearch#1774
Egavasyug pushed a commit to Egavasyug/hermes-agent that referenced this pull request Jun 10, 2026
NousResearch#1775)

* fix(session): skip corrupt lines in load_transcript instead of crashing

Wrap json.loads() in load_transcript() with try/except JSONDecodeError
so that partial JSONL lines (from mid-write crashes like OOM/SIGKILL)
are skipped with a warning instead of crashing the entire transcript
load. The rest of the history loads fine.

Adds a logger.warning with the session ID and truncated corrupt line
content for debugging visibility.

Salvaged from PR NousResearch#1193 by alireza78a.
Closes NousResearch#1193

* fix(stt): respect explicit provider config instead of env-var fallback

Rework _get_provider() to separate explicit config from auto-detect.
When stt.provider is explicitly set in config.yaml, that choice is
authoritative — no silent cross-provider fallback based on which env
vars happen to be set. When no provider is configured, auto-detect
still tries: local > groq > openai.

This fixes the reported scenario where provider: local + a placeholder
OPENAI_API_KEY caused the system to silently select OpenAI and fail
with a 401.

Closes NousResearch#1774
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant