Skip to content

security(terminal): strip VERTEX_CREDENTIALS_PATH/GOOGLE_APPLICATION_CREDENTIALS from subprocess env - #481

Open
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56582
Open

security(terminal): strip VERTEX_CREDENTIALS_PATH/GOOGLE_APPLICATION_CREDENTIALS from subprocess env#481
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56582

Conversation

@hashbender

Copy link
Copy Markdown
Owner

Summary

  • VERTEX_CREDENTIALS_PATH (added in the recent Vertex AI provider, OAuth2 service-account JSON path) and GOOGLE_APPLICATION_CREDENTIALS (the ADC fallback the adapter also reads) were never added to the subprocess-env sanitization blocklist, so they leak into every spawned subprocess (terminal, codex/copilot app-server, browser workers).
  • Root cause: Vertex authenticates via OAuth2, not the legacy PROVIDER_REGISTRY, so the registry-derived blocklist loop in _build_provider_env_blocklist() never sees it. Separately, VERTEX_CREDENTIALS_PATH is declared in OPTIONAL_ENV_VARS with password=False (it's a filesystem path, not a bare secret string) under category="provider" — a category the metadata-derived loop only checks for tool/messaging, or setting+password. It falls through both automatic sources.
  • This is the same leak class already closed for every other provider's credentials in harden(tools): block AUXILIARY_*_API_KEY and _BASE_URL from subprocess env NousResearch/hermes-agent#53503/fix(codex): route app-server subprocess env through hermes_subprocess_env NousResearch/hermes-agent#55709: a leaked value here discloses the on-disk location of a GCP service-account key to any subprocess a model-driving CLI or tool spawns.
  • Fix: add both var names to the hand-maintained blocked.update({...}) set in tools/environments/local.py, alongside the other non-registry provider vars (GOOGLE_API_KEY, XAI_API_KEY, etc.) that already live there for the same reason.

Test plan

  • Added test_vertex_credentials_path_is_stripped to tests/tools/test_local_env_blocklist.py, asserting both vars are stripped from subprocess env.
  • Ran the full existing tests/tools/test_local_env_blocklist.py + tests/tools/test_hermes_subprocess_env.py suites (68 tests) — all pass, no regressions.
  • Ran tests/agent/test_vertex_adapter.py + tests/hermes_cli/test_vertex_provider.py (23 tests) — all pass, confirming the adapter/config layer is unaffected.

Mirror-of: NousResearch#56582
NousResearch#56582

@tenki-reviewer

tenki-reviewer Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Complete

Files Reviewed: 2
Findings: 1

By Severity:

  • 🟡 Medium: 1

Two medium-severity security concerns in the env blocklist: a structural gap where OPTIONAL_ENV_VARS skips the 'provider' category, and GOOGLE_APPLICATION_CREDENTIALS blocking that contradicts the intentional AWS credential-chain preservation design.

Files Reviewed (2 files)
tests/tools/test_local_env_blocklist.py
tools/environments/local.py

@tenki-reviewer tenki-reviewer Bot 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.

Risk: 🟡 Medium (45/100) — 1 medium finding · 27 LOC across 2 files


PR #481 Review

This PR adds two GCP-related env vars (VERTEX_CREDENTIALS_PATH and GOOGLE_APPLICATION_CREDENTIALS) to the environment blocklist in tools/environments/local.py with corresponding tests.

Findings

finding-001 — OPTIONAL_ENV_VARS skips category='provider' (medium)

The _build_provider_env_blocklist() loop at tools/environments/local.py:134-143 iterates OPTIONAL_ENV_VARS but only handles categories tool, messaging, and setting — it intentionally skips provider. This means any future credential-path env var registered with category='provider'/password=False (the same pattern as VERTEX_CREDENTIALS_PATH) will bypass both automated blocklist sources and require manual enumeration in the hardcoded set. The fix for VERTEX_CREDENTIALS_PATH closes the immediate gap but the structural gap remains.

finding-002 — GOOGLE_APPLICATION_CREDENTIALS blocking contradicts AWS policy (medium)

The existing design at tools/environments/local.py:97-113 explicitly declares that general cloud-provider credential chains (including AWS_SHARED_CREDENTIALS_FILE — a filesystem path to a credentials file) are intentionally inheritable because "the local terminal is the user's trusted operator shell." GOOGLE_APPLICATION_CREDENTIALS is the GCP Application Default Credentials analogue of AWS_SHARED_CREDENTIALS_FILE. Blocking it breaks GCP SDK tools (gcloud, gsutil, bq, google.auth.default()) in the terminal, with no env_passthrough escape. Only VERTEX_CREDENTIALS_PATH (Hermes-managed, in OPTIONAL_ENV_VARS) should be blocked — analogous to AWS_BEARER_TOKEN_BEDROCK.

Recommendation

Remove GOOGLE_APPLICATION_CREDENTIALS from the hardcoded blocklist and add 'provider' category handling to the OPTIONAL_ENV_VARS loop so future credential-path secrets are auto-blocked without manual enumeration.

# Path to a GCP service-account JSON, not a bare key, so
# OPTIONAL_ENV_VARS marks it password=False and the loop above skips it.
"VERTEX_CREDENTIALS_PATH",
"GOOGLE_APPLICATION_CREDENTIALS",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 GOOGLE_APPLICATION_CREDENTIALS blocking contradicts AWS credential policy and breaks GCP SDK tools in the terminal (security)

The patch adds GOOGLE_APPLICATION_CREDENTIALS to _HERMES_PROVIDER_ENV_BLOCKLIST at local.py:161. This env var is the GCP Application Default Credentials path — the standard mechanism GCP SDKs use to locate service-account credentials. The existing design at local.py:97-113 explicitly declares that general cloud-provider credential chains (AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, AWS_SESSION_TOKEN, AWS_PROFILE, AWS_SHARED_CREDENTIALS_FILE, AWS_CONFIG_FILE, etc.) are INTENTIONALLY left inheritable because 'the local terminal is the user's trusted operator shell.' AWS_SHARED_CREDENTIALS_FILE (a filesystem path pointer to a credentials file) is the direct AWS analogue of GOOGLE_APPLICATION_CREDENTIALS — both are path pointers to cloud-credential files, and both are preserved for AWS but GOOGLE_APPLICATION_CREDENTIALS would be blocked under this patch. Unlike VERTEX_CREDENTIALS_PATH (which is Hermes-managed via OPTIONAL_ENV_VARS), GOOGLE_APPLICATION_CREDENTIALS is not in OPTIONAL_ENV_VARS and is a general user environment variable. Blocking it breaks any GCP SDK-based tool or script run in the terminal that relies on ADC, with no env_passthrough escape.

💡 Suggestion: Remove GOOGLE_APPLICATION_CREDENTIALS from the hardcoded blocklist at line 161. Only VERTEX_CREDENTIALS_PATH (which is Hermes-managed via OPTIONAL_ENV_VARS and is the direct Vertex AI credential pointer) should be in the blocklist. This is consistent with the AWS policy that blocks AWS_BEARER_TOKEN_BEDROCK (Hermes-specific) but preserves AWS_SHARED_CREDENTIALS_FILE (general cloud credential chain). Update the test to only assert VERTEX_CREDENTIALS_PATH is stripped and verify GOOGLE_APPLICATION_CREDENTIALS is preserved.

📋 Prompt for AI Agents

In tools/environments/local.py, remove 'GOOGLE_APPLICATION_CREDENTIALS' from the hardcoded blocklist set (line 161). Keep 'VERTEX_CREDENTIALS_PATH' (line 160). In tests/tools/test_local_env_blocklist.py, update test_vertex_credentials_path_is_stripped to: (a) only set and assert VERTEX_CREDENTIALS_PATH is stripped; (b) set GOOGLE_APPLICATION_CREDENTIALS and assert it IS preserved (matching the AWS_SHARED_CREDENTIALS_FILE preservation pattern at test_general_aws_credential_chain_is_preserved). The rationale: VERTEX_CREDENTIALS_PATH is a Hermes-managed credential pointer (in OPTIONAL_ENV_VARS), analogous to AWS_BEARER_TOKEN_BEDROCK which is correctly blocked. GOOGLE_APPLICATION_CREDENTIALS is the standard GCP ADC env var, analogous to AWS_SHARED_CREDENTIALS_FILE which the existing design intentionally preserves per local.py:97-116.

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