Skip to content

fix(kanban): strip parent credentials from worker env - #55600

Open
necoweb3 wants to merge 1 commit into
NousResearch:mainfrom
necoweb3:fix/kanban-worker-env-scope
Open

fix(kanban): strip parent credentials from worker env#55600
necoweb3 wants to merge 1 commit into
NousResearch:mainfrom
necoweb3:fix/kanban-worker-env-scope

Conversation

@necoweb3

Copy link
Copy Markdown
Contributor

Summary

This prevents Kanban worker subprocesses from inheriting provider and gateway credentials from the dispatching process environment.

_default_spawn() launches workers for the assignee profile by setting HERMES_HOME / HERMES_PROFILE, but it previously started from dict(os.environ). Because Hermes credential resolution checks process env before the profile .env, a worker intended to run under one profile could still see provider keys and internal gateway secrets from the dispatcher/default profile.

Why

Kanban workers are profile-scoped subprocesses. Their config, auth state, and .env should come from the assignee profile, not from whichever gateway/CLI process happened to dispatch the task.

Before this change, a dispatcher process containing values such as OPENAI_API_KEY, ANTHROPIC_API_KEY, AUXILIARY_*_API_KEY, or GATEWAY_RELAY_SECRET passed those values straight into the worker env. That can cause wrong-profile credential use and leaks internal gateway routing secrets into an unrelated worker process.

Changes

  • Build Kanban worker env from hermes_subprocess_env(inherit_credentials=False) instead of raw os.environ.
  • Keep the existing worker-specific pins for HERMES_HOME, HERMES_PROFILE, HERMES_KANBAN_*, workspace, branch, and runtime settings.
  • Strip dynamic Hermes internal secrets (AUXILIARY_*_API_KEY, AUXILIARY_*_BASE_URL, GATEWAY_RELAY_*) from the worker env.
  • Add regression coverage proving parent profile credentials do not reach _default_spawn() workers.

Tests

python -m pytest tests/hermes_cli/test_kanban_boards.py -k "default_spawn" -q --timeout-method=thread
2 passed, 55 deselected

python -m pytest tests/hermes_cli/test_kanban_db.py -k "dispatcher_spawn_injects_kanban_db_and_workspaces_root" -q --timeout-method=thread
1 passed, 222 deselected

python -m pytest tests/hermes_cli/test_kanban_core_functionality.py -k "default_spawn" -q --timeout-method=thread
6 passed, 163 deselected

python -m pytest tests/hermes_cli/test_kanban_worker_spawn_toolsets.py -q --timeout-method=thread
2 passed

@alt-glitch alt-glitch added type/security Security vulnerability or hardening comp/cron Cron scheduler and job management sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data P3 Low — cosmetic, nice to have labels Jun 30, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Approved

Security fix that strips parent profile credentials from kanban worker subprocess environments. Uses the centralized hermes_subprocess_env(inherit_credentials=False) and then explicitly removes GATEWAY_RELAY_SECRET, GATEWAY_RELAY_*, and AUXILIARY_*_API_KEY/AUXILIARY_*_BASE_URL variables. Well-tested.

Looks Good

  • Defense-in-depth: centralized env sanitizer + explicit credential stripping
  • Test verifies both credential removal and profile routing preservation
  • Clean separation of concerns

Reviewed by Hermes Agent

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for addressing this worker-profile credential boundary. Current origin/main still builds the worker environment with dict(os.environ) at hermes_cli/kanban_db.py:8085, so routing through hermes_subprocess_env(inherit_credentials=False) is a valid fix.

Problems

  • hermes_cli/kanban_db.py:7692 removes every GATEWAY_RELAY_* variable. The centralized sanitizer deliberately preserves non-secret routing hints such as GATEWAY_RELAY_URL and GATEWAY_RELAY_PLATFORMS (tools/environments/local.py:292-320; tests/tools/test_local_env_blocklist.py:691-730).

Suggested changes

  • Remove the extra manual filtering loop at hermes_cli/kanban_db.py:7691-7698; the centralized helper already strips provider keys, auxiliary secrets, relay authentication material, and relay identity when called with inherit_credentials=False.
  • Add a worker-spawn regression assertion that a non-secret relay routing hint remains available.

Automated hermes-sweeper review.

Comment thread hermes_cli/kanban_db.py
from tools.environments.local import hermes_subprocess_env
env = hermes_subprocess_env(inherit_credentials=False)
for key in list(env):
if key == "GATEWAY_RELAY_SECRET" or key.startswith("GATEWAY_RELAY_"):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please remove this broad GATEWAY_RELAY_* filter. hermes_subprocess_env(inherit_credentials=False) already removes relay authentication material, while the shared sanitizer intentionally preserves non-secret routing hints such as GATEWAY_RELAY_URL and GATEWAY_RELAY_PLATFORMS.

@andrexibiza

Copy link
Copy Markdown
Contributor

Bound to the child-process credential-inheritance class under #83565 (#83565) — same bug class, different surface. Kanban worker env strip; Wave E — shares hermes_cli/kanban_db.py with #83007. The EPIC carries the live class table, dedup adjudication, and the dependency-driven merge order.

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

Labels

comp/cron Cron scheduler and job management P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants