SECURITY: log secret masking fails open and can emit raw secrets when masking throws #171

New issue

Open

opened 2026-09-09 15:05:22 +00:00 by nsaspy · 0 comments

nsaspy commented

2026-09-09 15:05:22 +00:00

Owner

Finding

helpers/log.py::Log._mask_recursive() catches every exception from secret-manager lookup/masking/recursive processing and returns the original object unchanged:

try:
    ...
    return secrets_mgr.mask_values(...)
except Exception:
    return obj

The returned value is then placed into logs/state and can be sent to the WebUI or persisted. If the secret manager fails, is temporarily misconfigured, hits an unexpected value shape, or throws while a plugin/context is changing, the fallback is therefore the unmasked secret-bearing payload.

Secret redaction is a confidentiality boundary and should fail closed, not fail open.

Direction

Define a fail-closed masking contract:

distinguish lookup/setup failure from values known to contain no registered secrets only if that distinction is provable
on masking failure, do not emit the original candidate value to UI/persistence
replace with a safe redaction/error sentinel and log the masking failure through a channel that itself cannot contain the candidate secret
add metrics/diagnostics so failures are visible
avoid recursive masking implementation that can throw on one nested field and expose the entire original structure

Acceptance

Any exception in masking cannot cause the original candidate string/object to reach LogItem output.
Nested structures fail closed at the smallest safe unit without leaking siblings/parent raw values.
Masking failure diagnostics contain no candidate secret material.
Tests inject failures from secret-manager acquisition and mask_values() for strings/dicts/lists.
Tests cover state snapshots and persisted chat serialization after a masking failure.
Streaming secret fragments remain protected, coordinated with #32.
Existing normal masking behavior remains unchanged when the secrets manager is healthy.

Mirrored from lost-rob0t/a0-symbolics#33 via tracker sync.

## Finding `helpers/log.py::Log._mask_recursive()` catches **every exception** from secret-manager lookup/masking/recursive processing and returns the original object unchanged: ```python try: ... return secrets_mgr.mask_values(...) except Exception: return obj ``` The returned value is then placed into logs/state and can be sent to the WebUI or persisted. If the secret manager fails, is temporarily misconfigured, hits an unexpected value shape, or throws while a plugin/context is changing, the fallback is therefore the **unmasked secret-bearing payload**. Secret redaction is a confidentiality boundary and should fail closed, not fail open. ## Direction Define a fail-closed masking contract: - distinguish lookup/setup failure from values known to contain no registered secrets only if that distinction is provable - on masking failure, do not emit the original candidate value to UI/persistence - replace with a safe redaction/error sentinel and log the masking failure through a channel that itself cannot contain the candidate secret - add metrics/diagnostics so failures are visible - avoid recursive masking implementation that can throw on one nested field and expose the entire original structure ## Acceptance - [ ] Any exception in masking cannot cause the original candidate string/object to reach `LogItem` output. - [ ] Nested structures fail closed at the smallest safe unit without leaking siblings/parent raw values. - [ ] Masking failure diagnostics contain no candidate secret material. - [ ] Tests inject failures from secret-manager acquisition and `mask_values()` for strings/dicts/lists. - [ ] Tests cover state snapshots and persisted chat serialization after a masking failure. - [ ] Streaming secret fragments remain protected, coordinated with #32. - [ ] Existing normal masking behavior remains unchanged when the secrets manager is healthy. --- *Mirrored from [`lost-rob0t/a0-symbolics#33`](https://github.com/lost-rob0t/a0-symbolics/issues/33)* via tracker sync.