mirror of
https://github.com/rookiestar28/ComfyUI-OpenClaw.git
synced 2026-08-14 00:48:07 +00:00
feat(security): add optional 1password secret provider chain
This commit is contained in:
@@ -56,6 +56,7 @@ This project is designed to make **ComfyUI a reliable automation target** with a
|
||||
- Runtime profile startup hardening is fail-closed in hardened mode
|
||||
- Connector ingress is fail-closed in public/hardened posture when platform allowlists are missing, with synchronized startup gate, deployment-profile check, Security Doctor posture, and startup audit visibility
|
||||
- Admin write actions are protected by an explicit **Admin Token** boundary
|
||||
- Optional local secret-manager integration supports 1Password CLI with explicit enable + command allowlist fail-closed controls while keeping frontend surfaces secret-blind
|
||||
- Webhook ingress is **deny-by-default** until authentication is configured
|
||||
- Encrypted webhook ingress is **fail-closed** on signature/decrypt/app-id validation failures
|
||||
- Bridge worker ingress enforces device token auth, scope checks, and idempotency handling
|
||||
@@ -91,6 +92,17 @@ Deployment profiles and hardening checklists:
|
||||
|
||||
<details>
|
||||
|
||||
<summary><strong>Optional local secret-manager baseline for safer key sourcing</strong></summary>
|
||||
|
||||
- Added a pluggable backend secret-provider chain for API keys (`env -> optional 1Password CLI -> encrypted server store -> none`) so operators can keep runtime keys out of plaintext deployment config where needed.
|
||||
- Added fail-closed 1Password guardrails requiring explicit enablement, executable allowlist, command path validation, and bounded lookup timeout behavior.
|
||||
- Added regression coverage for precedence resolution, allowlist/failure fallback behavior, and no-secret-leak logging expectations.
|
||||
- Completed full verification gate pass on `dev` (detect-secrets, pre-commit, backend unit suites, adversarial/retry/real-backend lanes, and frontend Playwright E2E).
|
||||
|
||||
</details>
|
||||
|
||||
<details>
|
||||
|
||||
<summary><strong>Today’s implementation roundup across frontend quality, planner contracts, and connector security baselines</strong></summary>
|
||||
|
||||
- Completed the frontend quality bundle by stabilizing canonical style ownership, adding baseline frontend unit coverage, and expanding regression coverage for Library/Approvals/admin-console parity.
|
||||
|
||||
@@ -55,4 +55,3 @@ Phase 2+ (future follow-ups):
|
||||
- Rejected due to blast radius and rollback difficulty.
|
||||
2. Keep dual systems and patch ad hoc:
|
||||
- Rejected due to ongoing precedence drift and maintenance cost.
|
||||
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
# OpenClaw Config & Secrets Contract (v1)
|
||||
|
||||
> **Status**: normative
|
||||
> **Version**: 1.0.3
|
||||
> **Date**: 2026-03-01
|
||||
> **Version**: 1.0.4
|
||||
> **Date**: 2026-03-07
|
||||
|
||||
This document defines the authoritative configuration contract for OpenClaw. It enumerates all supported environment variables, their precedence rules, and security classifications.
|
||||
|
||||
@@ -31,6 +31,24 @@ Controls the core LLM client used by nodes (Planner, Refiner, etc.).
|
||||
| `OPENCLAW_LLM_BASE_URL` | No | Provider default | Override base URL (crucial for local/compatible providers). |
|
||||
| `OPENCLAW_LLM_TIMEOUT`| No | `120` | Request timeout in seconds. |
|
||||
|
||||
Optional local secret-manager path (S11, disabled by default):
|
||||
|
||||
| Variable | Required | Default | Description |
|
||||
| :--- | :--- | :--- | :--- |
|
||||
| `OPENCLAW_1PASSWORD_ENABLED` | No | `0` | Enables optional 1Password CLI provider for API key resolution. |
|
||||
| `OPENCLAW_1PASSWORD_ALLOWED_COMMANDS` | **Yes (when enabled)** | - | Comma-separated executable allowlist. Empty allowlist with enabled provider is fail-closed. |
|
||||
| `OPENCLAW_1PASSWORD_CMD` | No | `op` | 1Password CLI executable name/path. Must match allowlist entry. |
|
||||
| `OPENCLAW_1PASSWORD_VAULT` | **Yes (when enabled)** | - | Vault name used to resolve secret references. |
|
||||
| `OPENCLAW_1PASSWORD_ITEM_TEMPLATE` | No | `openclaw/{provider}` | Item path template. Must contain `{provider}` placeholder. |
|
||||
| `OPENCLAW_1PASSWORD_FIELD` | No | `api_key` | Item field name containing the key value. |
|
||||
| `OPENCLAW_1PASSWORD_TIMEOUT_SEC` | No | `5` | CLI lookup timeout (bounded, fail-closed on timeout). |
|
||||
|
||||
Lookup precedence for provider keys:
|
||||
1. Provider-specific env key (`OPENCLAW_*`, legacy `MOLTBOT_*`)
|
||||
2. Generic env key (`OPENCLAW_LLM_API_KEY`, legacy aliases)
|
||||
3. Optional 1Password provider (if enabled and allowlist-valid)
|
||||
4. Encrypted server-side secret store (`secrets.enc.json`)
|
||||
|
||||
**SSRF Protection:**
|
||||
|
||||
| Variable | Default | Description |
|
||||
|
||||
@@ -23,6 +23,7 @@ Users should audit these flags before deploying to a public or untrusted network
|
||||
| `OPENCLAW_BRIDGE_ENABLED` | `0` | **High** | Enables the sidecar bridge for remote orchestration. Requires `OPENCLAW_BRIDGE_DEVICE_TOKEN` (and in public posture also mTLS + device allowlist controls). |
|
||||
| `OPENCLAW_ALLOW_ANY_PUBLIC_LLM_HOST` | `0` | **High** | Bypasses the known-host allowlist for LLM `base_url`. Allows SSRF to public IPs. |
|
||||
| `OPENCLAW_ALLOW_INSECURE_BASE_URL` | `0` | **Critical** | Allows HTTP (non-HTTPS) or private IP `base_url` for LLM. Risk of internal network scanning (SSRF). |
|
||||
| `OPENCLAW_1PASSWORD_ENABLED` | `0` | **Medium** | Enables optional local 1Password CLI key lookup. Requires explicit command allowlist (`OPENCLAW_1PASSWORD_ALLOWED_COMMANDS`) and vault config; fail-closed when misconfigured. |
|
||||
| `OPENCLAW_LOG_TRUNCATE_ON_START` | `0` | **Low** | Operational log hygiene toggle. If `1`, truncates active `openclaw.log` at startup (once per process). |
|
||||
| `OPENCLAW_CONNECTOR_DISCORD_TOKEN` | *None* | **Medium** | Presence enables Discord Bot gateway. |
|
||||
| `OPENCLAW_CONNECTOR_LINE_CHANNEL_SECRET` | *None* | **Medium** | Presence enables LINE webhook listener. Requires a public HTTPS endpoint. |
|
||||
|
||||
@@ -8,6 +8,9 @@ This SOP defines operational procedures for three security-critical materials:
|
||||
|
||||
Use this runbook for routine rotation, emergency revocation, and disaster recovery.
|
||||
|
||||
Scope note:
|
||||
- If `S11` optional 1Password provider is enabled, provider API keys can be sourced from local 1Password instead of `secrets.enc.json`. In that mode, this SOP still governs the local encryption key lifecycle for any secrets that remain in server-side store.
|
||||
|
||||
## 1. State Artifacts
|
||||
|
||||
All paths are relative to `OPENCLAW_STATE_DIR` (legacy fallback: `MOLTBOT_STATE_DIR`):
|
||||
@@ -22,6 +25,12 @@ All paths are relative to `OPENCLAW_STATE_DIR` (legacy fallback: `MOLTBOT_STATE_
|
||||
Optional startup log hygiene for incident drills:
|
||||
- Set `OPENCLAW_LOG_TRUNCATE_ON_START=1` before restart when you need a clean `openclaw.log` timeline.
|
||||
|
||||
Optional `S11` local secret-manager settings (if used):
|
||||
- `OPENCLAW_1PASSWORD_ENABLED=1`
|
||||
- `OPENCLAW_1PASSWORD_ALLOWED_COMMANDS=<allowlisted executables>`
|
||||
- `OPENCLAW_1PASSWORD_CMD=op`
|
||||
- `OPENCLAW_1PASSWORD_VAULT=<vault>`
|
||||
|
||||
## 2. Global Rules
|
||||
|
||||
1. Always take a timestamped backup before any lifecycle operation.
|
||||
|
||||
@@ -154,4 +154,3 @@ def resolve_layered_config(
|
||||
sources[key] = source
|
||||
|
||||
return effective, sources
|
||||
|
||||
|
||||
+62
-56
@@ -1,12 +1,13 @@
|
||||
"""
|
||||
LLM Provider API Key Management.
|
||||
R16: Key lookup policy per provider with env var precedence.
|
||||
R16/S11: Key lookup policy per provider with pluggable secret-provider chain.
|
||||
"""
|
||||
|
||||
import logging
|
||||
import os
|
||||
from typing import Optional
|
||||
|
||||
from ..secret_providers import resolve_provider_secret
|
||||
from .catalog import PROVIDER_CATALOG, get_provider_info
|
||||
|
||||
logger = logging.getLogger("ComfyUI-OpenClaw.services.providers.keys")
|
||||
@@ -23,53 +24,16 @@ def get_api_key_for_provider(provider: str) -> Optional[str]:
|
||||
"""
|
||||
Get API key for a specific provider.
|
||||
|
||||
Precedence (S25):
|
||||
Precedence (S11/S25):
|
||||
1. Provider-specific env var (preferred: OPENCLAW_*; legacy: MOLTBOT_*)
|
||||
2. Generic key (OPENCLAW_LLM_API_KEY, MOLTBOT_LLM_API_KEY, CLAWDBOT_LLM_API_KEY)
|
||||
3. Server secret store (server-side persistence)
|
||||
2. Generic env key (OPENCLAW_LLM_API_KEY, MOLTBOT_LLM_API_KEY, CLAWDBOT_LLM_API_KEY)
|
||||
3. Optional 1Password CLI provider (explicit opt-in + allowlisted command)
|
||||
4. Server secret store (encrypted server-side persistence)
|
||||
|
||||
Returns None if no key found (acceptable for local providers).
|
||||
"""
|
||||
provider_info = get_provider_info(provider)
|
||||
|
||||
# Try provider-specific key first
|
||||
if provider_info and provider_info.env_key_name:
|
||||
candidates = []
|
||||
if provider_info.env_key_name.startswith("MOLTBOT_"):
|
||||
candidates.append(
|
||||
provider_info.env_key_name.replace("MOLTBOT_", "OPENCLAW_", 1)
|
||||
)
|
||||
candidates.append(provider_info.env_key_name)
|
||||
for env_name in candidates:
|
||||
key = os.environ.get(env_name)
|
||||
if key:
|
||||
return key
|
||||
|
||||
# Fall back to generic keys
|
||||
for env_name in GENERIC_KEY_NAMES:
|
||||
key = os.environ.get(env_name)
|
||||
if key:
|
||||
return key
|
||||
|
||||
# S25: Fall back to server secret store (best-effort; empty store = no effect).
|
||||
try:
|
||||
from ..secret_store import get_secret_store
|
||||
|
||||
store = get_secret_store()
|
||||
|
||||
# Try provider-specific secret
|
||||
key = store.get_secret(provider)
|
||||
if key:
|
||||
return key
|
||||
|
||||
# Try generic secret
|
||||
key = store.get_secret("generic")
|
||||
if key:
|
||||
return key
|
||||
except Exception as e:
|
||||
logger.debug(f"S25: Failed to check secret store (non-fatal): {e}")
|
||||
|
||||
return None
|
||||
key, _source = resolve_provider_secret(provider)
|
||||
return key
|
||||
|
||||
|
||||
def requires_api_key(provider: str) -> bool:
|
||||
@@ -100,7 +64,7 @@ def get_all_configured_keys() -> dict:
|
||||
"""
|
||||
result = {}
|
||||
|
||||
# Check server secret store (best-effort)
|
||||
# Check server secret store status (best-effort)
|
||||
store_status = {}
|
||||
try:
|
||||
from ..secret_store import get_secret_store
|
||||
@@ -112,19 +76,45 @@ def get_all_configured_keys() -> dict:
|
||||
|
||||
for provider_id, info in PROVIDER_CATALOG.items():
|
||||
if info.env_key_name:
|
||||
key = os.environ.get(info.env_key_name)
|
||||
key = None
|
||||
provider_candidates = []
|
||||
if info.env_key_name.startswith("MOLTBOT_"):
|
||||
provider_candidates.append(
|
||||
info.env_key_name.replace("MOLTBOT_", "OPENCLAW_", 1)
|
||||
)
|
||||
provider_candidates.append(info.env_key_name)
|
||||
|
||||
# Determine source
|
||||
source = None
|
||||
if key:
|
||||
source = "env"
|
||||
elif provider_id in store_status:
|
||||
for env_name in provider_candidates:
|
||||
value = os.environ.get(env_name)
|
||||
if value:
|
||||
key = value
|
||||
source = "env"
|
||||
break
|
||||
|
||||
if key is None:
|
||||
for env_name in GENERIC_KEY_NAMES:
|
||||
value = os.environ.get(env_name)
|
||||
if value:
|
||||
key = value
|
||||
source = "env"
|
||||
break
|
||||
|
||||
if key is None and (
|
||||
provider_id in store_status or "generic" in store_status
|
||||
):
|
||||
source = "server_store"
|
||||
|
||||
if key is None and source is None:
|
||||
resolved, resolved_source = resolve_provider_secret(provider_id)
|
||||
if resolved:
|
||||
key = resolved
|
||||
source = resolved_source
|
||||
|
||||
result[provider_id] = {
|
||||
"env_var": info.env_key_name,
|
||||
"configured": key is not None or provider_id in store_status,
|
||||
"masked": mask_api_key(key) if key else None,
|
||||
"configured": key is not None or source is not None,
|
||||
"masked": mask_api_key(key) if key and source == "env" else None,
|
||||
"source": source,
|
||||
}
|
||||
else:
|
||||
@@ -135,13 +125,29 @@ def get_all_configured_keys() -> dict:
|
||||
"source": "local",
|
||||
}
|
||||
|
||||
# Add generic secret if stored
|
||||
if "generic" in store_status:
|
||||
# Add generic key status for diagnostics.
|
||||
generic_key = None
|
||||
generic_source = None
|
||||
for env_name in GENERIC_KEY_NAMES:
|
||||
value = os.environ.get(env_name)
|
||||
if value:
|
||||
generic_key = value
|
||||
generic_source = "env"
|
||||
break
|
||||
if generic_key is None and "generic" in store_status:
|
||||
generic_source = "server_store"
|
||||
if generic_key is None and generic_source is None:
|
||||
resolved, resolved_source = resolve_provider_secret("generic")
|
||||
if resolved:
|
||||
generic_key = resolved
|
||||
generic_source = resolved_source
|
||||
|
||||
if generic_source is not None:
|
||||
result["generic"] = {
|
||||
"env_var": "OPENCLAW_LLM_API_KEY",
|
||||
"configured": True,
|
||||
"masked": None, # Never expose stored secrets
|
||||
"source": "server_store",
|
||||
"masked": mask_api_key(generic_key) if generic_source == "env" else None,
|
||||
"source": generic_source,
|
||||
}
|
||||
|
||||
return result
|
||||
|
||||
@@ -0,0 +1,301 @@
|
||||
"""
|
||||
S11 secret provider chain.
|
||||
|
||||
Provides deterministic API-key resolution via pluggable providers:
|
||||
- env
|
||||
- optional 1Password CLI
|
||||
- encrypted server-side secret store
|
||||
- default none
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
import os
|
||||
import re
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
from typing import Optional, Protocol
|
||||
|
||||
try:
|
||||
from .config_layers import GENERIC_LLM_API_KEY_ENV_KEYS, get_preferred_env_value
|
||||
except ImportError:
|
||||
from services.config_layers import ( # type: ignore
|
||||
GENERIC_LLM_API_KEY_ENV_KEYS,
|
||||
get_preferred_env_value,
|
||||
)
|
||||
|
||||
try:
|
||||
from .providers.catalog import get_provider_info
|
||||
except ImportError:
|
||||
from services.providers.catalog import get_provider_info # type: ignore
|
||||
|
||||
logger = logging.getLogger("ComfyUI-OpenClaw.services.secret_providers")
|
||||
|
||||
_TRUTHY = {"1", "true", "yes", "on"}
|
||||
_PROVIDER_ID_RE = re.compile(r"^[a-z0-9][a-z0-9._-]{0,63}$")
|
||||
_VAULT_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$")
|
||||
_FIELD_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$")
|
||||
_ITEM_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._/-]{0,127}$")
|
||||
|
||||
|
||||
def _is_truthy(value: Optional[str]) -> bool:
|
||||
if value is None:
|
||||
return False
|
||||
return value.strip().lower() in _TRUTHY
|
||||
|
||||
|
||||
def _env_value(
|
||||
primary: str, legacy: str, default: Optional[str] = None
|
||||
) -> Optional[str]:
|
||||
value, _used_legacy = get_preferred_env_value(primary, legacy)
|
||||
if value is None:
|
||||
return default
|
||||
return value
|
||||
|
||||
|
||||
class SecretProvider(Protocol):
|
||||
source: str
|
||||
|
||||
def get_secret(self, provider: str) -> Optional[str]: ...
|
||||
|
||||
|
||||
class EnvSecretProvider:
|
||||
source = "env"
|
||||
|
||||
def _provider_env_candidates(self, provider: str) -> list[str]:
|
||||
info = get_provider_info(provider)
|
||||
if not info or not info.env_key_name:
|
||||
return []
|
||||
|
||||
candidates: list[str] = []
|
||||
if info.env_key_name.startswith("MOLTBOT_"):
|
||||
candidates.append(info.env_key_name.replace("MOLTBOT_", "OPENCLAW_", 1))
|
||||
candidates.append(info.env_key_name)
|
||||
return candidates
|
||||
|
||||
def get_secret(self, provider: str) -> Optional[str]:
|
||||
# Provider-specific first
|
||||
for env_name in self._provider_env_candidates(provider):
|
||||
value = os.environ.get(env_name)
|
||||
if value:
|
||||
return value
|
||||
|
||||
# Generic fallback
|
||||
for env_name in GENERIC_LLM_API_KEY_ENV_KEYS:
|
||||
value = os.environ.get(env_name)
|
||||
if value:
|
||||
return value
|
||||
return None
|
||||
|
||||
|
||||
class OnePasswordSecretProvider:
|
||||
source = "onepassword"
|
||||
|
||||
def _enabled(self) -> bool:
|
||||
return _is_truthy(
|
||||
_env_value("OPENCLAW_1PASSWORD_ENABLED", "MOLTBOT_1PASSWORD_ENABLED", "0")
|
||||
)
|
||||
|
||||
def _command(self) -> str:
|
||||
command = _env_value("OPENCLAW_1PASSWORD_CMD", "MOLTBOT_1PASSWORD_CMD", "op")
|
||||
return str(command or "").strip()
|
||||
|
||||
def _allowed_commands(self) -> set[str]:
|
||||
raw = _env_value(
|
||||
"OPENCLAW_1PASSWORD_ALLOWED_COMMANDS",
|
||||
"MOLTBOT_1PASSWORD_ALLOWED_COMMANDS",
|
||||
"",
|
||||
)
|
||||
return {
|
||||
token.strip().lower()
|
||||
for token in str(raw or "").split(",")
|
||||
if token and token.strip()
|
||||
}
|
||||
|
||||
def _vault(self) -> str:
|
||||
return str(
|
||||
_env_value("OPENCLAW_1PASSWORD_VAULT", "MOLTBOT_1PASSWORD_VAULT", "") or ""
|
||||
).strip()
|
||||
|
||||
def _item_template(self) -> str:
|
||||
return str(
|
||||
_env_value(
|
||||
"OPENCLAW_1PASSWORD_ITEM_TEMPLATE",
|
||||
"MOLTBOT_1PASSWORD_ITEM_TEMPLATE",
|
||||
"openclaw/{provider}",
|
||||
)
|
||||
or ""
|
||||
).strip()
|
||||
|
||||
def _field(self) -> str:
|
||||
return str(
|
||||
_env_value("OPENCLAW_1PASSWORD_FIELD", "MOLTBOT_1PASSWORD_FIELD", "api_key")
|
||||
or ""
|
||||
).strip()
|
||||
|
||||
def _timeout_sec(self) -> float:
|
||||
raw = _env_value(
|
||||
"OPENCLAW_1PASSWORD_TIMEOUT_SEC", "MOLTBOT_1PASSWORD_TIMEOUT_SEC", "5"
|
||||
)
|
||||
try:
|
||||
value = float(raw or "5")
|
||||
except (TypeError, ValueError):
|
||||
value = 5.0
|
||||
return max(1.0, min(value, 30.0))
|
||||
|
||||
def is_available(self) -> bool:
|
||||
if not self._enabled():
|
||||
return False
|
||||
|
||||
allowed = self._allowed_commands()
|
||||
if not allowed:
|
||||
# CRITICAL: explicit allowlist is required when 1Password provider is enabled.
|
||||
logger.warning(
|
||||
"S11: 1Password enabled but OPENCLAW_1PASSWORD_ALLOWED_COMMANDS is empty; fail-closed."
|
||||
)
|
||||
return False
|
||||
|
||||
command = self._command()
|
||||
if not command:
|
||||
logger.warning("S11: 1Password command is empty; fail-closed.")
|
||||
return False
|
||||
command_name = Path(command).name.lower()
|
||||
if command_name not in allowed:
|
||||
logger.warning(
|
||||
"S11: 1Password command '%s' not in allowlist; fail-closed.",
|
||||
command_name,
|
||||
)
|
||||
return False
|
||||
|
||||
vault = self._vault()
|
||||
field = self._field()
|
||||
template = self._item_template()
|
||||
if not vault or not _VAULT_RE.fullmatch(vault):
|
||||
logger.warning("S11: 1Password vault is missing/invalid; fail-closed.")
|
||||
return False
|
||||
if not field or not _FIELD_RE.fullmatch(field):
|
||||
logger.warning("S11: 1Password field is invalid; fail-closed.")
|
||||
return False
|
||||
if "{provider}" not in template:
|
||||
logger.warning(
|
||||
"S11: 1Password item template must include '{provider}'; fail-closed."
|
||||
)
|
||||
return False
|
||||
return True
|
||||
|
||||
def _build_ref(self, provider: str) -> Optional[str]:
|
||||
if not _PROVIDER_ID_RE.fullmatch(provider):
|
||||
logger.warning(
|
||||
"S11: Invalid provider id for 1Password lookup; fail-closed."
|
||||
)
|
||||
return None
|
||||
template = self._item_template()
|
||||
item = template.format(provider=provider)
|
||||
if not item or not _ITEM_RE.fullmatch(item) or ".." in item:
|
||||
logger.warning("S11: 1Password item name is invalid; fail-closed.")
|
||||
return None
|
||||
return f"op://{self._vault()}/{item}/{self._field()}"
|
||||
|
||||
def _read_ref(self, ref: str, provider: str) -> Optional[str]:
|
||||
command = self._command()
|
||||
args = [command, "read", ref]
|
||||
try:
|
||||
completed = subprocess.run(
|
||||
args,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
shell=False,
|
||||
timeout=self._timeout_sec(),
|
||||
check=False,
|
||||
)
|
||||
except FileNotFoundError:
|
||||
logger.warning("S11: 1Password command not found; fail-closed.")
|
||||
return None
|
||||
except subprocess.TimeoutExpired:
|
||||
logger.warning(
|
||||
"S11: 1Password lookup timed out for provider '%s'; fail-closed.",
|
||||
provider,
|
||||
)
|
||||
return None
|
||||
except Exception as exc:
|
||||
logger.warning(
|
||||
"S11: 1Password lookup error for provider '%s' (%s); fail-closed.",
|
||||
provider,
|
||||
type(exc).__name__,
|
||||
)
|
||||
return None
|
||||
|
||||
if completed.returncode != 0:
|
||||
logger.warning(
|
||||
"S11: 1Password lookup failed for provider '%s' (exit=%s); fail-closed.",
|
||||
provider,
|
||||
completed.returncode,
|
||||
)
|
||||
return None
|
||||
|
||||
value = (completed.stdout or "").strip()
|
||||
return value or None
|
||||
|
||||
def get_secret(self, provider: str) -> Optional[str]:
|
||||
if not self.is_available():
|
||||
return None
|
||||
|
||||
provider_ref = self._build_ref(provider)
|
||||
if provider_ref:
|
||||
value = self._read_ref(provider_ref, provider)
|
||||
if value:
|
||||
return value
|
||||
|
||||
if provider != "generic":
|
||||
generic_ref = self._build_ref("generic")
|
||||
if generic_ref:
|
||||
return self._read_ref(generic_ref, "generic")
|
||||
return None
|
||||
|
||||
|
||||
class ServerStoreSecretProvider:
|
||||
source = "server_store"
|
||||
|
||||
def get_secret(self, provider: str) -> Optional[str]:
|
||||
try:
|
||||
from .secret_store import get_secret_store
|
||||
except ImportError:
|
||||
from services.secret_store import get_secret_store # type: ignore
|
||||
|
||||
try:
|
||||
store = get_secret_store()
|
||||
value = store.get_secret(provider)
|
||||
if value:
|
||||
return value
|
||||
if provider != "generic":
|
||||
return store.get_secret("generic")
|
||||
except Exception as exc:
|
||||
logger.debug(
|
||||
"S25/S11: secret store lookup failed (non-fatal): %s",
|
||||
exc,
|
||||
)
|
||||
return None
|
||||
|
||||
|
||||
def get_secret_providers() -> list[SecretProvider]:
|
||||
"""Return key-resolution providers in deterministic order."""
|
||||
return [
|
||||
EnvSecretProvider(),
|
||||
OnePasswordSecretProvider(),
|
||||
ServerStoreSecretProvider(),
|
||||
]
|
||||
|
||||
|
||||
def resolve_provider_secret(provider: str) -> tuple[Optional[str], Optional[str]]:
|
||||
"""
|
||||
Resolve a provider secret through the configured provider chain.
|
||||
|
||||
Returns:
|
||||
(secret, source) where source in {"env","onepassword","server_store"} or None.
|
||||
"""
|
||||
for resolver in get_secret_providers():
|
||||
secret = resolver.get_secret(provider)
|
||||
if secret:
|
||||
return secret, resolver.source
|
||||
return None, None
|
||||
@@ -24,7 +24,10 @@ class TestR139ConfigLayers(unittest.TestCase):
|
||||
value, used_legacy = get_preferred_env_value(
|
||||
"OPENCLAW_LLM_PROVIDER",
|
||||
"MOLTBOT_LLM_PROVIDER",
|
||||
env={"OPENCLAW_LLM_PROVIDER": "openai", "MOLTBOT_LLM_PROVIDER": "anthropic"},
|
||||
env={
|
||||
"OPENCLAW_LLM_PROVIDER": "openai",
|
||||
"MOLTBOT_LLM_PROVIDER": "anthropic",
|
||||
},
|
||||
)
|
||||
self.assertEqual(value, "openai")
|
||||
self.assertFalse(used_legacy)
|
||||
@@ -106,4 +109,3 @@ class TestR139ConfigLayers(unittest.TestCase):
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
|
||||
@@ -0,0 +1,136 @@
|
||||
"""
|
||||
S11 secret-provider chain tests.
|
||||
"""
|
||||
|
||||
import os
|
||||
import shutil
|
||||
import tempfile
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
from services.providers.keys import get_all_configured_keys, get_api_key_for_provider
|
||||
from services.secret_store import get_secret_store
|
||||
|
||||
|
||||
class TestS11SecretProviderChain(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self._orig_env = dict(os.environ)
|
||||
self._tmpdir = tempfile.mkdtemp()
|
||||
# Keep environment deterministic per test.
|
||||
for key in list(os.environ.keys()):
|
||||
if key.startswith("OPENCLAW_") or key.startswith("MOLTBOT_"):
|
||||
del os.environ[key]
|
||||
get_secret_store(state_dir=self._tmpdir).clear_all()
|
||||
|
||||
def tearDown(self):
|
||||
os.environ.clear()
|
||||
os.environ.update(self._orig_env)
|
||||
shutil.rmtree(self._tmpdir, ignore_errors=True)
|
||||
|
||||
def test_env_precedence_over_onepassword_and_store(self):
|
||||
os.environ["OPENCLAW_OPENAI_API_KEY"] = "sk-env-priority"
|
||||
os.environ["OPENCLAW_1PASSWORD_ENABLED"] = "1"
|
||||
os.environ["OPENCLAW_1PASSWORD_ALLOWED_COMMANDS"] = "op"
|
||||
os.environ["OPENCLAW_1PASSWORD_VAULT"] = "vlt"
|
||||
|
||||
store = get_secret_store(state_dir=self._tmpdir)
|
||||
store.set_secret("openai", "sk-store")
|
||||
|
||||
with patch("services.secret_providers.subprocess.run") as mock_run:
|
||||
key = get_api_key_for_provider("openai")
|
||||
|
||||
self.assertEqual(key, "sk-env-priority")
|
||||
mock_run.assert_not_called()
|
||||
|
||||
def test_onepassword_lookup_success(self):
|
||||
os.environ["OPENCLAW_1PASSWORD_ENABLED"] = "1"
|
||||
os.environ["OPENCLAW_1PASSWORD_ALLOWED_COMMANDS"] = "op"
|
||||
os.environ["OPENCLAW_1PASSWORD_VAULT"] = "teamvault"
|
||||
|
||||
with patch("services.secret_providers.subprocess.run") as mock_run:
|
||||
mock_run.return_value = type(
|
||||
"R",
|
||||
(),
|
||||
{"returncode": 0, "stdout": "sk-op-123\n", "stderr": ""},
|
||||
)()
|
||||
key = get_api_key_for_provider("openai")
|
||||
|
||||
self.assertEqual(key, "sk-op-123")
|
||||
args = mock_run.call_args.args[0]
|
||||
self.assertEqual(args[:2], ["op", "read"])
|
||||
self.assertIn("op://teamvault/openclaw/openai/api_key", args)
|
||||
|
||||
def test_onepassword_allowlist_fail_closed_falls_back_to_store(self):
|
||||
os.environ["OPENCLAW_1PASSWORD_ENABLED"] = "1"
|
||||
os.environ["OPENCLAW_1PASSWORD_ALLOWED_COMMANDS"] = "safeop"
|
||||
os.environ["OPENCLAW_1PASSWORD_CMD"] = "op"
|
||||
os.environ["OPENCLAW_1PASSWORD_VAULT"] = "teamvault"
|
||||
|
||||
store = get_secret_store(state_dir=self._tmpdir)
|
||||
store.set_secret("openai", "sk-store-only")
|
||||
|
||||
with patch("services.secret_providers.subprocess.run") as mock_run:
|
||||
key = get_api_key_for_provider("openai")
|
||||
|
||||
self.assertEqual(key, "sk-store-only")
|
||||
mock_run.assert_not_called()
|
||||
|
||||
def test_onepassword_failure_falls_back_to_generic_store(self):
|
||||
os.environ["OPENCLAW_1PASSWORD_ENABLED"] = "1"
|
||||
os.environ["OPENCLAW_1PASSWORD_ALLOWED_COMMANDS"] = "op"
|
||||
os.environ["OPENCLAW_1PASSWORD_VAULT"] = "teamvault"
|
||||
|
||||
store = get_secret_store(state_dir=self._tmpdir)
|
||||
store.set_secret("generic", "sk-generic-store")
|
||||
|
||||
with patch("services.secret_providers.subprocess.run") as mock_run:
|
||||
mock_run.return_value = type(
|
||||
"R",
|
||||
(),
|
||||
{"returncode": 1, "stdout": "", "stderr": "unauthorized"},
|
||||
)()
|
||||
key = get_api_key_for_provider("openai")
|
||||
|
||||
self.assertEqual(key, "sk-generic-store")
|
||||
self.assertGreaterEqual(mock_run.call_count, 1)
|
||||
|
||||
def test_onepassword_error_log_does_not_leak_secret_value(self):
|
||||
os.environ["OPENCLAW_1PASSWORD_ENABLED"] = "1"
|
||||
os.environ["OPENCLAW_1PASSWORD_ALLOWED_COMMANDS"] = "op"
|
||||
os.environ["OPENCLAW_1PASSWORD_VAULT"] = "teamvault"
|
||||
|
||||
leaked = "sk-should-not-appear"
|
||||
with patch("services.secret_providers.subprocess.run") as mock_run:
|
||||
mock_run.return_value = type(
|
||||
"R",
|
||||
(),
|
||||
{"returncode": 1, "stdout": "", "stderr": f"bad key {leaked}"},
|
||||
)()
|
||||
with self.assertLogs(
|
||||
"ComfyUI-OpenClaw.services.secret_providers", level="WARNING"
|
||||
) as logs:
|
||||
key = get_api_key_for_provider("openai")
|
||||
|
||||
self.assertIsNone(key)
|
||||
self.assertNotIn(leaked, "\n".join(logs.output))
|
||||
|
||||
def test_configured_keys_reports_onepassword_source_without_value(self):
|
||||
os.environ["OPENCLAW_1PASSWORD_ENABLED"] = "1"
|
||||
os.environ["OPENCLAW_1PASSWORD_ALLOWED_COMMANDS"] = "op"
|
||||
os.environ["OPENCLAW_1PASSWORD_VAULT"] = "teamvault"
|
||||
|
||||
def _resolver(*_args, **_kwargs):
|
||||
return type(
|
||||
"R", (), {"returncode": 0, "stdout": "sk-op-xyz", "stderr": ""}
|
||||
)()
|
||||
|
||||
with patch("services.secret_providers.subprocess.run", side_effect=_resolver):
|
||||
info = get_all_configured_keys()
|
||||
|
||||
self.assertTrue(info["openai"]["configured"])
|
||||
self.assertEqual(info["openai"]["source"], "onepassword")
|
||||
self.assertIsNone(info["openai"]["masked"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user