diff --git a/CHANGELOG.md b/CHANGELOG.md index 91b291513..b6dca9945 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,13 @@ Changed `wf10_batch.robot` to be less likely to create files, and `plan_generation_graph.robot` to give more test answers. ## [Unreleased] +- **Action Schema Env Var Exfiltration** (#9089): Restricted `_interpolate_env_vars` / + `_env_replacer` in `src/cleveragents/action/schema.py` to only interpolate environment + variables prefixed with `CLEVERAGENTS_` and with at least one character after the prefix. + Previously, any `${VAR}` placeholder in action YAML could exfiltrate arbitrary environment + variables (e.g., `AWS_SECRET_ACCESS_KEY`, `DATABASE_PASSWORD`). Disallowed variables are + now left as literal `${VAR}` placeholders. Added Behave BDD coverage in + `features/action_schema_env_var_security.feature`. - **fix(cli/plan): plan correct JSON output envelope fix and BDD test coverage** (#8584 / PR #8662): Restructured `agents plan correct --format json` output to nest correction fields under `data.correction` (e.g., `data.correction.mode`) and populate the spec-required CLI envelope with `command="plan correct"`, `status`, `exit_code`, `timing`, and `messages` fields. Added three BDD scenarios in `features/tdd_plan_correct_json_output.feature` validating the envelope structure for both revert and append modes. - **fix(cli): add --url flag to resource add for git resource type** (#6322): Added support for the `--url` flag on `agents resource add git` command, allowing users to specify a remote URL for git resources. The flag is validated to only apply to git resource types. Includes Behave BDD tests in `features/resource_cli_git_url_flag.feature` and Robot Framework integration tests verifying correct URL validation and CLI behavior. - **Session create JSON envelope** (#6441): Fixed `agents session create --format json` returning a flat `data` dict instead of the spec-required nested structure with `data.session`, `data.settings`, and `data.actor_details` sub-objects. The `command` field is now populated correctly. Extended JSON envelope coverage to `agents session list`, `show`, `delete --format json`, `export --output-format json`, and `import --format json` so all session commands emit a structured `messages[].text` field (`"0 sessions listed"`, `"Session details loaded"`, `"Session deleted"`, `"Export completed"`, `"Import completed"`). diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index b683984b1..d26cd04b8 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -30,6 +30,7 @@ Below are some of the specific details of various contributions. * HAMZA KHYARI has contributed the ACMS execute-phase context assembler project-level hot_max_tokens fix (PR #11036 / issue #11035): added `_resolve_effective_budget()` method that reads each linked project's `settings.hot_max_tokens` and uses the maximum override value as the pipeline budget instead of the hardcoded global 16K default. * HAL 9000 has contributed the automated CLI docstring example validation (#9106): added `DocstringExampleValidator` to enforce positional-before-option ordering in CLI `Examples:` sections, with Behave test coverage and CONTRIBUTING.md documentation. * HAL 9000 has contributed the AutoDebugAgent prompt injection mitigation fix (#9110): sanitized user-provided `error_message` and `code_context` fields in all three agent methods using `PromptSanitizer` boundary markers, added graceful `PromptInjectionDetected` exception handling, and added BDD and Robot Framework integration tests for the security fix. +* HAL 9000 has contributed the action schema environment variable exfiltration fix (#9089): restricted `_env_replacer` in `src/cleveragents/action/schema.py` to only interpolate variables prefixed with `CLEVERAGENTS_` (with at least one character after the prefix), preventing exfiltration of sensitive credentials via action YAML configurations. * This project was made possible thanks to considerable donation of time, money, and resources by CleverThis, Inc. * HAL 9000 has contributed automated bug fixes, CLI output formatting improvements, and ongoing maintenance as part of the CleverAgents automation system. * HAL 9000 has contributed the pr-review-pool-supervisor tracking prefix documentation fix (#7891): aligned all documentation references from the outdated `AUTO-REV-POOL` prefix to the correct `AUTO-REV-SUP` prefix used in production. diff --git a/features/action_schema_env_var_security.feature b/features/action_schema_env_var_security.feature new file mode 100644 index 000000000..40e5f08f1 --- /dev/null +++ b/features/action_schema_env_var_security.feature @@ -0,0 +1,118 @@ +@action @security +Feature: Action Schema Environment Variable Security + As a security-conscious developer + I want to ensure that action YAML files can only interpolate allowlisted environment variables + So that sensitive credentials cannot be exfiltrated through malicious action configurations + + Background: + Given an action YAML config with only required fields + + # ============================================================ + # Allowed Environment Variable Interpolation + # ============================================================ + + Scenario: Interpolate CLEVERAGENTS_ prefixed environment variable + Given an environment variable "CLEVERAGENTS_API_KEY" set to "secret-key-123" + And an action YAML config with description "${CLEVERAGENTS_API_KEY}" + When I load the action config YAML + Then the action config description should be "secret-key-123" + + Scenario: Interpolate multiple CLEVERAGENTS_ prefixed variables + Given an environment variable "CLEVERAGENTS_HOST" set to "localhost" + And an environment variable "CLEVERAGENTS_PORT" set to "8080" + And an action YAML config with description "Connect to ${CLEVERAGENTS_HOST}:${CLEVERAGENTS_PORT}" + When I load the action config YAML + Then the action config description should be "Connect to localhost:8080" + + Scenario: Missing CLEVERAGENTS_ variable left as placeholder + Given no environment variable "CLEVERAGENTS_MISSING_VAR" + And an action YAML config with description "Config: ${CLEVERAGENTS_MISSING_VAR}" + When I load the action config YAML + Then the action config description should be "Config: ${CLEVERAGENTS_MISSING_VAR}" + + # ============================================================ + # Disallowed Environment Variable Protection + # ============================================================ + + Scenario: Disallow AWS_SECRET_ACCESS_KEY interpolation + Given an environment variable "AWS_SECRET_ACCESS_KEY" set to "aws-secret-123" + And an action YAML config with description "Secret: ${AWS_SECRET_ACCESS_KEY}" + When I load the action config YAML + Then the action config description should be "Secret: ${AWS_SECRET_ACCESS_KEY}" + And the action config description should NOT contain "aws-secret-123" + + Scenario: Disallow DATABASE_PASSWORD interpolation + Given an environment variable "DATABASE_PASSWORD" set to "db-password-456" + And an action YAML config with description "DB: ${DATABASE_PASSWORD}" + When I load the action config YAML + Then the action config description should be "DB: ${DATABASE_PASSWORD}" + And the action config description should NOT contain "db-password-456" + + Scenario: Disallow API_KEY interpolation + Given an environment variable "API_KEY" set to "api-key-789" + And an action YAML config with description "API: ${API_KEY}" + When I load the action config YAML + Then the action config description should be "API: ${API_KEY}" + And the action config description should NOT contain "api-key-789" + + Scenario: Disallow HOME interpolation + Given an environment variable "HOME" set to "/home/user" + And an action YAML config with description "Home: ${HOME}" + When I load the action config YAML + Then the action config description should be "Home: ${HOME}" + And the action config description should NOT contain "/home/user" + + Scenario: Disallow PATH interpolation + Given an environment variable "PATH" set to "/usr/bin:/bin" + And an action YAML config with description "Path: ${PATH}" + When I load the action config YAML + Then the action config description should be "Path: ${PATH}" + And the action config description should NOT contain "/usr/bin" + + # ============================================================ + # Mixed Allowed and Disallowed Variables + # ============================================================ + + Scenario: Mix allowed and disallowed variables + Given an environment variable "CLEVERAGENTS_CONFIG" set to "allowed-config" + And an environment variable "SECRET_TOKEN" set to "secret-token-xyz" + And an action YAML config with description "Config: ${CLEVERAGENTS_CONFIG}, Token: ${SECRET_TOKEN}" + When I load the action config YAML + Then the action config description should be "Config: allowed-config, Token: ${SECRET_TOKEN}" + And the action config description should NOT contain "secret-token-xyz" + + # ============================================================ + # Edge Cases + # ============================================================ + + Scenario: Bare prefix not interpolated (suffix required) + Given an environment variable "CLEVERAGENTS_" set to "empty-prefix" + And an action YAML config with description "Var: ${CLEVERAGENTS_}" + When I load the action config YAML + Then the action config description should be "Var: ${CLEVERAGENTS_}" + + Scenario: Case sensitivity - lowercase not allowed + Given an environment variable "cleveragents_api_key" set to "lowercase-key" + And an action YAML config with description "Key: ${cleveragents_api_key}" + When I load the action config YAML + Then the action config description should be "Key: ${cleveragents_api_key}" + And the action config description should NOT contain "lowercase-key" + + Scenario: Interpolation in nested fields + Given an environment variable "CLEVERAGENTS_ACTOR" set to "openai/gpt-4" + And an action YAML config with strategy_actor "${CLEVERAGENTS_ACTOR}" + When I load the action config YAML + Then the action config strategy_actor should be "openai/gpt-4" + + Scenario: Interpolation in list fields + Given an environment variable "CLEVERAGENTS_INVARIANT" set to "No secrets in code" + And an action YAML config with invariants containing "${CLEVERAGENTS_INVARIANT}" + When I load the action config YAML + Then the action config invariants should contain "No secrets in code" + + Scenario: Disallowed variable in list fields + Given an environment variable "SECRET_INVARIANT" set to "Secret invariant" + And an action YAML config with invariants containing "${SECRET_INVARIANT}" + When I load the action config YAML + Then the action config invariants should contain "${SECRET_INVARIANT}" + And the action config invariants should NOT contain "Secret invariant" diff --git a/features/steps/action_schema_env_var_security_steps.py b/features/steps/action_schema_env_var_security_steps.py new file mode 100644 index 000000000..1c5fc3870 --- /dev/null +++ b/features/steps/action_schema_env_var_security_steps.py @@ -0,0 +1,156 @@ +"""Step definitions for action schema environment variable security tests. + +Tests for features/action_schema_env_var_security.feature — validates that +the ActionConfigSchema only allows interpolation of CLEVERAGENTS_ prefixed +environment variables, preventing exfiltration of sensitive credentials. + +Step phrasing is qualified with "action config" / "action config YAML" to +avoid the AmbiguousStep collisions that would otherwise occur with +action_schema_steps.py, action_persistence_steps.py, and +action_model_branch_coverage_steps.py. +""" + +from __future__ import annotations + +import json +import os + +from behave import given, then, when # type: ignore[import-untyped] +from behave.runner import Context # type: ignore[import-untyped] + +from cleveragents.action.schema import ActionConfigSchema + +# ──────────────────────────────────────────────────────────── +# Helpers +# ──────────────────────────────────────────────────────────── + +_MINIMAL_YAML = """\ +name: local/simple-action +description: A simple action for testing +strategy_actor: openai/gpt-4 +execution_actor: openai/gpt-4 +definition_of_done: All tests pass +""" + + +def _yaml_scalar(value: str) -> str: + """Render a Python string as a YAML double-quoted scalar. + + JSON string encoding (``"..."`` with backslash escapes) is a valid + subset of YAML flow scalars, so it round-trips safely for arbitrary + test data — including descriptions that contain ``: `` (which would + otherwise be parsed as a nested mapping) or ``${VAR}`` placeholders. + """ + return json.dumps(value) + + +# ──────────────────────────────────────────────────────────── +# Given steps +# ──────────────────────────────────────────────────────────── + + +@given("an action YAML config with only required fields") +def step_given_minimal_yaml(context: Context) -> None: + """Provide a minimal valid action YAML string.""" + context.action_yaml_string = _MINIMAL_YAML + + +@given('an environment variable "{var_name}" set to "{var_value}"') +def step_given_env_var_set(context: Context, var_name: str, var_value: str) -> None: + """Set an environment variable for testing.""" + os.environ[var_name] = var_value + if not hasattr(context, "env_vars_to_clean"): + context.env_vars_to_clean = [] + context.env_vars_to_clean.append(var_name) + + +@given('no environment variable "{var_name}"') +def step_given_no_env_var(context: Context, var_name: str) -> None: + """Ensure an environment variable is not set.""" + if var_name in os.environ: + del os.environ[var_name] + + +@given('an action YAML config with description "{description}"') +def step_given_yaml_with_description(context: Context, description: str) -> None: + """Provide YAML with a custom description value.""" + context.action_yaml_string = _MINIMAL_YAML.replace( + "A simple action for testing", _yaml_scalar(description) + ) + + +@given('an action YAML config with strategy_actor "{actor}"') +def step_given_yaml_with_strategy_actor(context: Context, actor: str) -> None: + """Provide YAML with a custom strategy_actor value.""" + context.action_yaml_string = _MINIMAL_YAML.replace( + "openai/gpt-4", _yaml_scalar(actor), 1 + ) + + +@given('an action YAML config with invariants containing "{invariant}"') +def step_given_yaml_with_invariants(context: Context, invariant: str) -> None: + """Provide YAML with invariants containing a specific value.""" + context.action_yaml_string = ( + _MINIMAL_YAML + f"\ninvariants:\n - {_yaml_scalar(invariant)}\n" + ) + + +# ──────────────────────────────────────────────────────────── +# When steps +# ──────────────────────────────────────────────────────────── + + +@when("I load the action config YAML") +def step_when_load_action_yaml(context: Context) -> None: + """Load and validate the action YAML.""" + try: + context.action_config = ActionConfigSchema.from_yaml(context.action_yaml_string) + context.load_error = None + except Exception as e: + context.action_config = None + context.load_error = e + + +# ──────────────────────────────────────────────────────────── +# Then steps +# ──────────────────────────────────────────────────────────── + + +@then('the action config description should be "{expected_description}"') +def step_then_description_is(context: Context, expected_description: str) -> None: + """Verify the action description matches the expected value.""" + assert context.action_config is not None, "Action config should be loaded" + assert context.action_config.description == expected_description, ( + f"Expected description '{expected_description}', " + f"got '{context.action_config.description}'" + ) + + +@then('the action config description should NOT contain "{text}"') +def step_then_description_not_contains(context: Context, text: str) -> None: + """Verify the action description does not contain the specified text.""" + assert context.action_config is not None, "Action config should be loaded" + assert text not in context.action_config.description, ( + f"Description should not contain '{text}', " + f"but got '{context.action_config.description}'" + ) + + +@then('the action config invariants should contain "{expected_invariant}"') +def step_then_invariants_contain(context: Context, expected_invariant: str) -> None: + """Verify the action invariants contain the expected value.""" + assert context.action_config is not None, "Action config should be loaded" + assert expected_invariant in context.action_config.invariants, ( + f"Expected invariant '{expected_invariant}' not found " + f"in {context.action_config.invariants}" + ) + + +@then('the action config invariants should NOT contain "{text}"') +def step_then_invariants_not_contain(context: Context, text: str) -> None: + """Verify the action invariants do not contain the specified text.""" + assert context.action_config is not None, "Action config should be loaded" + for invariant in context.action_config.invariants: + assert text not in invariant, ( + f"Invariant should not contain '{text}', but got '{invariant}'" + ) diff --git a/src/cleveragents/action/schema.py b/src/cleveragents/action/schema.py index 0d5070cd0..b94877419 100644 --- a/src/cleveragents/action/schema.py +++ b/src/cleveragents/action/schema.py @@ -45,6 +45,9 @@ NAMESPACED_NAME_RE = re.compile(r"^[a-z0-9][a-z0-9_-]*/[a-z0-9][a-z0-9_-]*$") #: Pattern for ``${VAR}`` environment variable references. _ENV_VAR_RE = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*)\}") +#: Allowed prefix for environment variable interpolation (security restriction). +_ALLOWED_ENV_VAR_PREFIX = "CLEVERAGENTS_" + #: Supported action states (draft excluded from YAML configs). _VALID_STATES = frozenset({"available", "archived"}) @@ -449,6 +452,10 @@ def _interpolate_env_vars(data: dict[str, Any]) -> dict[str, Any]: Only string values are interpolated. Missing environment variables are left as-is (no error) to allow deferred resolution. + + Security: Only environment variables with the ``CLEVERAGENTS_`` prefix + are eligible for interpolation. Attempts to reference other variables + are left as literal ``${VAR}`` placeholders. """ result: dict[str, Any] = {} for key, value in data.items(): @@ -475,6 +482,15 @@ def _interpolate_env_vars(data: dict[str, Any]) -> dict[str, Any]: def _env_replacer(match: re.Match[str]) -> str: - """Replace a single ``${VAR}`` match with its env value.""" + """Replace a single ``${VAR}`` match with its env value. + + Security: Only allows interpolation of environment variables with the + ``CLEVERAGENTS_`` prefix. Other variables are left as literal placeholders. + """ var_name = match.group(1) - return os.environ.get(var_name, match.group(0)) + if var_name.startswith(_ALLOWED_ENV_VAR_PREFIX) and len(var_name) > len( + _ALLOWED_ENV_VAR_PREFIX + ): + return os.environ.get(var_name, match.group(0)) + # Return the original placeholder for disallowed variables + return match.group(0)