cleveragents/cleveragents-core

Fork 3

BUG-HUNT: [concurrency] shared/redaction.py _show_secrets global flag is module-level state shared across threads, causing race conditions in concurrent log processing #7388

New issue

Open

opened 2026-04-10 18:47:47 +00:00 by HAL9000 · 3 comments

HAL9000 commented

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

Owner

Bug Report: [concurrency] redaction.py _show_secrets global flag mutation and reading is not atomic, allowing race conditions in concurrent logging

Severity Assessment

Impact: The _show_secrets global flag can be read by logging threads while being written by a main thread (e.g., during --debug --show-secrets CLI invocation). Although the individual flag read/write is protected by _flag_lock, the structlog processor secrets_masking_processor() calls get_show_secrets() on every log event. In a multi-threaded server deployment, a race between set_show_secrets(True) and the processor creates a window where some log events are masked and others are not, leading to inconsistent log output.
Likelihood: Medium — affects server deployments with multi-threaded logging where the --show-secrets flag can be toggled
Priority: Medium

Location

File: src/cleveragents/shared/redaction.py
Function/Class: set_show_secrets(), get_show_secrets(), secrets_masking_processor()
Lines: ~70-120

Description

The _show_secrets flag uses a _flag_lock for thread-safe reads/writes:

_show_secrets: bool = False
_flag_lock = threading.Lock()

def get_show_secrets() -> bool:
    with _flag_lock:
        return _show_secrets

def set_show_secrets(value: bool) -> None:
    global _show_secrets
    with _flag_lock:
        _show_secrets = value

However, the secrets_masking_processor() structlog processor:

def secrets_masking_processor(...):
    if get_show_secrets():  # Lock acquired, read, lock released
        return event_dict  # RACE: by the time we return, flag may have changed
    
    result: dict[str, Any] = {}
    for key, value in event_dict.items():
        # ... redaction happens here ...

The issue is that between get_show_secrets() returning False and the actual redaction of the log event, another thread could call set_show_secrets(True), but the processor has already decided to redact. This creates an inconsistent log state where:

Some log events from before the flag change are redacted
Some log events after the flag change are not redacted (or vice versa)

For a security tool, inconsistent secret masking is a concern — an operator might enable --show-secrets for debugging expecting to see secrets, but some log events might still be masked.

Additionally, the _SECRET_PATTERNS list uses _patterns_lock for reads but the global _show_secrets uses a separate _flag_lock. This means the two locks are independent and can be held independently, which is correct but means the behavior between the two mechanisms is not atomic (a log event could be processed with one value of show_secrets and a different set of patterns).

Evidence

_show_secrets: bool = False  # Module-level global
_flag_lock = threading.Lock()  # Separate lock from _patterns_lock

def get_show_secrets() -> bool:
    with _flag_lock:
        return _show_secrets  # Lock released after this!

def secrets_masking_processor(logger, method_name, event_dict):
    if get_show_secrets():  # Window between here...
        return event_dict
    # ...and here where flag could change!
    result: dict[str, Any] = {}
    for key, value in event_dict.items():
        # This redaction could happen even though flag is now True

Expected Behavior

For a security tool, the show_secrets flag check and the subsequent redaction decision should be atomic relative to flag changes. The simplest fix is to capture the flag value once and use it consistently throughout the processor:

def secrets_masking_processor(logger, method_name, event_dict):
    # Capture the flag value once atomically
    show = get_show_secrets()  
    if show:
        return event_dict
    # Use the captured value, not re-checking
    # ... redaction logic ...

This is already what the code does — the issue is that get_show_secrets() returns a snapshot that's immediately stale.

Actual Behavior

The show_secrets flag is checked once per log event, and the result can be stale by the time redaction happens, leading to inconsistent log output in concurrent environments.

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: [concurrency] redaction.py _show_secrets global flag mutation and reading is not atomic, allowing race conditions in concurrent logging ### Severity Assessment - **Impact**: The `_show_secrets` global flag can be read by logging threads while being written by a main thread (e.g., during `--debug --show-secrets` CLI invocation). Although the individual flag read/write is protected by `_flag_lock`, the `structlog` processor `secrets_masking_processor()` calls `get_show_secrets()` on every log event. In a multi-threaded server deployment, a race between `set_show_secrets(True)` and the processor creates a window where some log events are masked and others are not, leading to inconsistent log output. - **Likelihood**: Medium — affects server deployments with multi-threaded logging where the `--show-secrets` flag can be toggled - **Priority**: Medium ### Location - **File**: `src/cleveragents/shared/redaction.py` - **Function/Class**: `set_show_secrets()`, `get_show_secrets()`, `secrets_masking_processor()` - **Lines**: ~70-120 ### Description The `_show_secrets` flag uses a `_flag_lock` for thread-safe reads/writes: ```python _show_secrets: bool = False _flag_lock = threading.Lock() def get_show_secrets() -> bool: with _flag_lock: return _show_secrets def set_show_secrets(value: bool) -> None: global _show_secrets with _flag_lock: _show_secrets = value ``` However, the `secrets_masking_processor()` structlog processor: ```python def secrets_masking_processor(...): if get_show_secrets(): # Lock acquired, read, lock released return event_dict # RACE: by the time we return, flag may have changed result: dict[str, Any] = {} for key, value in event_dict.items(): # ... redaction happens here ... ``` The issue is that between `get_show_secrets()` returning `False` and the actual redaction of the log event, another thread could call `set_show_secrets(True)`, but the processor has already decided to redact. This creates an inconsistent log state where: - Some log events from before the flag change are redacted - Some log events after the flag change are not redacted (or vice versa) For a security tool, inconsistent secret masking is a concern — an operator might enable `--show-secrets` for debugging expecting to see secrets, but some log events might still be masked. Additionally, the `_SECRET_PATTERNS` list uses `_patterns_lock` for reads but the global `_show_secrets` uses a separate `_flag_lock`. This means the two locks are independent and can be held independently, which is correct but means the behavior between the two mechanisms is not atomic (a log event could be processed with one value of `show_secrets` and a different set of patterns). ### Evidence ```python _show_secrets: bool = False # Module-level global _flag_lock = threading.Lock() # Separate lock from _patterns_lock def get_show_secrets() -> bool: with _flag_lock: return _show_secrets # Lock released after this! def secrets_masking_processor(logger, method_name, event_dict): if get_show_secrets(): # Window between here... return event_dict # ...and here where flag could change! result: dict[str, Any] = {} for key, value in event_dict.items(): # This redaction could happen even though flag is now True ``` ### Expected Behavior For a security tool, the show_secrets flag check and the subsequent redaction decision should be atomic relative to flag changes. The simplest fix is to capture the flag value once and use it consistently throughout the processor: ```python def secrets_masking_processor(logger, method_name, event_dict): # Capture the flag value once atomically show = get_show_secrets() if show: return event_dict # Use the captured value, not re-checking # ... redaction logic ... ``` This is already what the code does — the issue is that `get_show_secrets()` returns a snapshot that's immediately stale. ### Actual Behavior The `show_secrets` flag is checked once per log event, and the result can be stale by the time redaction happens, leading to inconsistent log output in concurrent environments. ### Category concurrency ### 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:51:41 +00:00

[AUTO-BUG-SUP] Bug Hunt Status (Cycle 4) #7384

HAL9000 referenced this issue

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

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

HAL9000 added the

State

Unverified

label

2026-04-10 19:35:09 +00:00

freemo added the

labels

2026-04-12 18:35:31 +00:00

HAL9000 added

and removed

labels

2026-04-14 06:24:07 +00:00

HAL9000 commented

2026-04-14 06:24:07 +00:00

Author

Owner

✅ Verified — Concurrency bug: _show_secrets global flag shared across threads — race condition in log processing. MoSCoW: Must-have. Priority: High.

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

✅ **Verified** — Concurrency bug: _show_secrets global flag shared across threads — race condition in log processing. MoSCoW: Must-have. Priority: High. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor

HAL9000 commented

2026-04-14 06:29:45 +00:00

Author

Owner

✅ Verified — Concurrency bug: _show_secrets global flag shared across threads — race condition in log processing. MoSCoW: Must-have. Priority: High.

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

HAL9000 commented

2026-04-14 06:36:14 +00:00

Author

Owner

✅ Verified — Concurrency bug: _show_secrets global flag shared across threads — race condition in log processing. MoSCoW: Must-have. Priority: High.

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

HAL9000 referenced this issue

2026-05-05 19:32:53 +00:00

feat(decisions): verify agents plan tree CLI command implementation #8691