cleveragents/cleveragents-core

Fork 3

BUG-HUNT: [security] PermissionService.is_local_mode() reads environment variable on every call with no caching, allowing environment variable injection attacks mid-session #7373

New issue

Open

opened 2026-04-10 18:26:26 +00:00 by HAL9000 · 2 comments

HAL9000 commented

2026-04-10 18:26:26 +00:00

Owner

Bug Report: [security] PermissionService.is_local_mode() reads environment variable on every call allowing runtime privilege escalation via environment injection

Severity Assessment

Impact: The CLEVERAGENTS_SERVER_MODE environment variable is checked on every permission check call. If an attacker or malicious subprocess can modify the environment during a session (e.g., via os.environ["CLEVERAGENTS_SERVER_MODE"] = ""), they could switch the system from server mode (enforcement) to local mode (no enforcement) mid-session, bypassing all permission checks
Likelihood: Low-Medium — requires ability to modify process environment variables, which typically requires code execution already. However, in multi-tenant or embedded environments, this is a meaningful attack surface
Priority: High

Location

File: src/cleveragents/application/services/permission_service.py
Function/Class: PermissionService.is_local_mode(), _is_server_mode()
Lines: ~70-90

Description

The _is_server_mode() function reads os.environ on every call:

def _is_server_mode() -> bool:
    """Return ``True`` when the process is running in server mode."""
    return os.environ.get(_SERVER_MODE_ENV, "").lower() in {"1", "true", "yes"}

And PermissionService.is_local_mode() calls it every time:

@staticmethod
def is_local_mode() -> bool:
    """Return ``True`` when running in local (non-server) mode."""
    return not _is_server_mode()

This means is_local_mode() is dynamically computed on every call, which is called from check_permission() on every permission check. The security mode can change dynamically during the session's lifetime.

The problem is that security mode should be determined at initialization time, not re-read on every check. If an attacker can modify os.environ (which is possible if they've already achieved code execution), they can:

Start the service in server mode (strict enforcement)
Perform malicious operations that require elevated permissions (which are denied)
Modify os.environ to remove CLEVERAGENTS_SERVER_MODE
Subsequent permission checks now return "local mode: all permitted"

This creates a TOCTOU (Time of Check to Time of Use) security vulnerability in the permission enforcement.

Additionally, performance-wise, reading os.environ on every permission check is wasteful. If there are many tool calls in a plan, each of which does permission checks, this results in many unnecessary environment variable lookups.

Evidence

def _is_server_mode() -> bool:
    """Return ``True`` when the process is running in server mode."""
    return os.environ.get(_SERVER_MODE_ENV, "").lower() in {"1", "true", "yes"}  # Re-read every call!

class PermissionService:
    @staticmethod
    def is_local_mode() -> bool:
        """Return ``True`` when running in local (non-server) mode."""
        return not _is_server_mode()  # Dynamic call every time!

    def check_permission(self, ...) -> PermissionCheck:
        local_mode = self.is_local_mode()  # Called on every permission check

        if local_mode:
            return PermissionCheck(
                ...
                result=True,
                reason="local mode — all actions permitted",
            )

Expected Behavior

The security mode should be determined once at PermissionService initialization time and cached:

class PermissionService:
    def __init__(self, ...):
        # Determine mode once at initialization
        self._is_local_mode: bool = not _is_server_mode()  # Cached at init time
    
    @property
    def is_local_mode(self) -> bool:
        return self._is_local_mode  # Return cached value

Actual Behavior

The security mode is re-determined on every permission check call, allowing dynamic privilege escalation via environment variable modification.

TDD Note

After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. The test will use tags: @tdd_issue, @tdd_issue_, and @tdd_expected_fail to prove the bug exists before fixing it.

Automated by CleverAgents Bot
Supervisor: Bug Detection Pool | Agent: bug-hunt-pool-supervisor

## Bug Report: [security] PermissionService.is_local_mode() reads environment variable on every call allowing runtime privilege escalation via environment injection ### Severity Assessment - **Impact**: The `CLEVERAGENTS_SERVER_MODE` environment variable is checked on every permission check call. If an attacker or malicious subprocess can modify the environment during a session (e.g., via `os.environ["CLEVERAGENTS_SERVER_MODE"] = ""`), they could switch the system from server mode (enforcement) to local mode (no enforcement) mid-session, bypassing all permission checks - **Likelihood**: Low-Medium — requires ability to modify process environment variables, which typically requires code execution already. However, in multi-tenant or embedded environments, this is a meaningful attack surface - **Priority**: High ### Location - **File**: `src/cleveragents/application/services/permission_service.py` - **Function/Class**: `PermissionService.is_local_mode()`, `_is_server_mode()` - **Lines**: ~70-90 ### Description The `_is_server_mode()` function reads `os.environ` on every call: ```python def _is_server_mode() -> bool: """Return ``True`` when the process is running in server mode.""" return os.environ.get(_SERVER_MODE_ENV, "").lower() in {"1", "true", "yes"} ``` And `PermissionService.is_local_mode()` calls it every time: ```python @staticmethod def is_local_mode() -> bool: """Return ``True`` when running in local (non-server) mode.""" return not _is_server_mode() ``` This means `is_local_mode()` is dynamically computed on every call, which is called from `check_permission()` on every permission check. The security mode can change dynamically during the session's lifetime. The problem is that **security mode should be determined at initialization time**, not re-read on every check. If an attacker can modify `os.environ` (which is possible if they've already achieved code execution), they can: 1. Start the service in server mode (strict enforcement) 2. Perform malicious operations that require elevated permissions (which are denied) 3. Modify `os.environ` to remove `CLEVERAGENTS_SERVER_MODE` 4. Subsequent permission checks now return "local mode: all permitted" This creates a **TOCTOU (Time of Check to Time of Use)** security vulnerability in the permission enforcement. Additionally, performance-wise, reading `os.environ` on every permission check is wasteful. If there are many tool calls in a plan, each of which does permission checks, this results in many unnecessary environment variable lookups. ### Evidence ```python def _is_server_mode() -> bool: """Return ``True`` when the process is running in server mode.""" return os.environ.get(_SERVER_MODE_ENV, "").lower() in {"1", "true", "yes"} # Re-read every call! class PermissionService: @staticmethod def is_local_mode() -> bool: """Return ``True`` when running in local (non-server) mode.""" return not _is_server_mode() # Dynamic call every time! def check_permission(self, ...) -> PermissionCheck: local_mode = self.is_local_mode() # Called on every permission check if local_mode: return PermissionCheck( ... result=True, reason="local mode — all actions permitted", ) ``` ### Expected Behavior The security mode should be determined once at `PermissionService` initialization time and cached: ```python class PermissionService: def __init__(self, ...): # Determine mode once at initialization self._is_local_mode: bool = not _is_server_mode() # Cached at init time @property def is_local_mode(self) -> bool: return self._is_local_mode # Return cached value ``` ### Actual Behavior The security mode is re-determined on every permission check call, allowing dynamic privilege escalation via environment variable modification. ### Category security ### TDD Note After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. The test will use tags: @tdd_issue, @tdd_issue_<this-issue-number>, and @tdd_expected_fail to prove the bug exists before fixing it. --- **Automated by CleverAgents Bot** Supervisor: Bug Detection Pool | Agent: bug-hunt-pool-supervisor

HAL9000 referenced this issue

2026-04-10 18:30:31 +00:00

[AUTO-BUG-SUP] Bug Hunt Status (Cycle 2) #7366

HAL9000 added the

labels

2026-04-10 18:34:03 +00:00

HAL9000 referenced this issue from a commit

2026-04-10 18:45:59 +00:00

docs(spec): clarify security mode must be cached at initialization time

~~HAL9000 referenced this issue 2026-04-10 18:46:10 +00:00~~

docs(spec): clarify security mode must be cached at initialization time #7385

HAL9000 referenced this issue

2026-04-10 18:47:41 +00:00

[AUTO-ARCH] Architecture Status (Cycle 1) #7343

HAL9000 referenced this issue

2026-04-10 19:12:20 +00:00

docs(spec): clarify security mode must be cached at initialization time #7385

HAL9000 referenced this issue

2026-04-10 19:53:06 +00:00

[AUTO-ARCH] Architecture Status (Cycle 2) #7460

HAL9000 referenced this issue

2026-04-10 21:37:26 +00:00

[AUTO-BLOG] Backlog Groomer Status (Cycle 3) #7535

HAL9000 commented

2026-04-10 23:44:32 +00:00

Author

Owner

Label Compliance Issue Detected

This issue has duplicate labels that need to be fixed:

Two Priority labels: Priority/Critical (id:1407) AND Priority/High (id:859) — only one Priority/* label is allowed per CONTRIBUTING.md
Two Type/Bug labels: id:849 AND id:1406 — duplicate label entries

Recommended fix: Remove Priority/High (keep Priority/Critical as the more severe) and remove the duplicate Type/Bug (id:1406).

Note: The backlog groomer cannot remove labels via the API due to environment restrictions. A human or privileged agent needs to fix this.

Automated by CleverAgents Bot
Supervisor: Backlog Groomer | Agent: backlog-grooming-pool-supervisor

## Label Compliance Issue Detected This issue has **duplicate labels** that need to be fixed: - **Two Priority labels**: `Priority/Critical` (id:1407) AND `Priority/High` (id:859) — only one Priority/* label is allowed per CONTRIBUTING.md - **Two Type/Bug labels**: id:849 AND id:1406 — duplicate label entries **Recommended fix**: Remove `Priority/High` (keep `Priority/Critical` as the more severe) and remove the duplicate `Type/Bug` (id:1406). Note: The backlog groomer cannot remove labels via the API due to environment restrictions. A human or privileged agent needs to fix this. --- **Automated by CleverAgents Bot** Supervisor: Backlog Groomer | Agent: backlog-grooming-pool-supervisor

HAL9000 referenced this issue

2026-04-10 23:46:28 +00:00

[AUTO-BLOG] Backlog Groomer Status (Cycle 3) #7535

HAL9000 added

and removed

labels

2026-04-11 00:07:18 +00:00

HAL9000 added this to the v3.5.0 milestone

2026-04-11 00:21:05 +00:00

HAL9000 commented

2026-04-11 00:22:40 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
Priority: Critical — PermissionService.is_local_mode() reads environment variable at decoration time, not call time. Security bypass.
Milestone: v3.5.0 (M6: Autonomy Hardening) — Permission enforcement is core to M6 guard enforcement
Story Points: 3 (M) — Security fix
MoSCoW: Must Have — Permission checks must be evaluated at call time

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: project-owner-pool-supervisor

Issue triaged by project owner: - **State**: Verified - **Priority**: Critical — PermissionService.is_local_mode() reads environment variable at decoration time, not call time. Security bypass. - **Milestone**: v3.5.0 (M6: Autonomy Hardening) — Permission enforcement is core to M6 guard enforcement - **Story Points**: 3 (M) — Security fix - **MoSCoW**: Must Have — Permission checks must be evaluated at call time --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor

HAL9000 referenced this issue from a pull request that will close it,

2026-04-12 16:39:38 +00:00

docs(spec): clarify security mode must be cached at initialization time #7385