Skip to content

fix(openviking): resolve 403 with API key auth and late .env loading - #21136

Closed
baofuen wants to merge 1 commit into
NousResearch:mainfrom
baobaoProject:fix/openviking-auth-headers
Closed

fix(openviking): resolve 403 with API key auth and late .env loading#21136
baofuen wants to merge 1 commit into
NousResearch:mainfrom
baobaoProject:fix/openviking-auth-headers

Conversation

@baofuen

@baofuen baofuen commented May 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Two bugs in plugins/memory/openviking/__init__.py prevent the OpenViking memory plugin from working.

Bug 1: X-OpenViking-Account header conflicts with API key auth (Blocker)

_headers() unconditionally sent X-OpenViking-Account/User/Agent headers alongside X-API-Key. When using API key authentication, the account is embedded in the key itself — the OpenViking server rejects explicit tenant header overrides with PERMISSION_DENIED (403).

Fix: Only send tenant headers in local-dev mode (no API key). When self._api_key is set, omit X-OpenViking-* headers.

Bug 2: Env vars captured once, not re-read after /reload (Medium)

initialize() snapshots os.environ.get("OPENVIKING_API_KEY", "") into self._api_key and creates _VikingClient once. /reload updates os.environ but the provider is not re-initialized. Even /new does not call initialize() again.

Fix: Add _ensure_client() that re-reads os.environ on every tool call and recreates the client when API key/endpoint changes. Applied in handle_tool_call() and system_prompt_block().

How to test

  1. Set memory.provider: openviking in config.yaml
  2. Configure .env with OPENVIKING_ENDPOINT and OPENVIKING_API_KEY
  3. Start hermes — viking_browse, viking_search, viking_read should return data instead of 403

Platforms tested

  • Linux (WSL2 Ubuntu)

Related

Bug 1: _headers() sent X-OpenViking-Account/User/Agent headers
alongside X-API-Key, causing PERMISSION_DENIED (403) for
API-key-authenticated servers. The account is embedded in the
key itself — explicit tenant headers are rejected.

Fix: Only send tenant headers in local-dev mode (no API key).
When self._api_key is set, omit X-OpenViking-* headers.

Bug 2: initialize() captured env vars once and never re-read them.
/reload or late .env additions had no effect without process restart.

Fix: Add _ensure_client() that re-reads os.environ on every tool
call and recreates the client when API key/endpoint changes.
Applied in handle_tool_call() and system_prompt_block().

Closes: NousResearch#21130
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins tool/memory Memory tool and memory providers labels May 7, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Competing PR: #21138 fixes the same two bugs from #21130 with a broader changeset (includes tests). These should be reviewed together — only one should be merged.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for identifying the OpenViking reload failure. The remaining reload problem is real on current main, but this patch needs current-main salvage rather than a direct merge.

Problems

  • The header portion is already covered by current main: plugins/memory/openviking/__init__.py:244-257 omits account/user headers for API-key requests, with coverage at tests/plugins/memory/test_openviking_provider.py:1762-1776.
  • Current initialization resolves settings through _resolve_connection_settings(_load_hermes_openviking_config()) at plugins/memory/openviking/__init__.py:2133-2139; the proposed direct os.environ refresh would bypass current ovcli/config resolution.
  • Current main has further cached-client gates in prefetch() (:2225-2229), sync_turn() (:3055-3058), session lifecycle hooks, and on_memory_write() (:3247-3256) that this patch does not refresh.
  • The PR contains no regression tests, and GitHub reports it as conflicting against current main.

Suggested changes

  • Preserve the current resolver and health/runtime-start behavior, refresh all relevant live access paths, and add reload/rebuild/no-rebuild regressions.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 13, 2026
@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Closing as superseded by #51952: the late .env/auth-refresh problem this PR targeted is now covered by the _ensure_client() implementation there (config re-resolved on each access, client rebuilt when OPENVIKING_* values change after /reload), and the 403/tenant-identity handling is covered by the structural trusted-mode retry matching. Thanks for the report and the fix, @baofuen — the underlying issue #21130 is addressed by that PR.

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

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data tool/memory Memory tool and memory providers type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OpenViking plugin: 403 with API key auth + env vars not reloaded after /reload

4 participants