Prevent self-inspection from leaking configured secrets
MyTool blocks direct access to sensitive nested paths, but its formatter still printed scalar fields for small config objects. That let `my(action="check", key="web_config.search")` expose `api_key` in plain text even though the docs promise sensitive sub-fields are protected. This keeps the change narrow: sensitive nested config fields are omitted from MyTool's formatted output, and regression coverage locks the behavior in. Constraint: Must preserve existing read-only inspection behavior for non-sensitive fields Constraint: Keep scope limited to MyTool rather than introducing broader redaction plumbing Rejected: Rework global context/tool redaction around MyTool | broader than needed for the leak path Confidence: high Scope-risk: narrow Reversibility: clean Directive: If more nested config rendering is added later, filter sensitive field names at the formatter boundary as well as the path resolver Tested: PYTHONPATH=$PWD pytest -q tests/agent/tools/test_self_tool.py /Users/jh0927/Workspace/nanobot-validation-artifacts-2026-04-18/test_my_tool_secret_leak_regression.py Not-tested: Full repository test suite Related: #3259
This commit is contained in:
@@ -67,6 +67,13 @@ class MyTool(Tool):
|
|||||||
"private_key", "access_token", "refresh_token", "auth",
|
"private_key", "access_token", "refresh_token", "auth",
|
||||||
})
|
})
|
||||||
|
|
||||||
|
@classmethod
|
||||||
|
def _is_sensitive_field_name(cls, name: str) -> bool:
|
||||||
|
lowered = name.lower()
|
||||||
|
return lowered in cls._SENSITIVE_NAMES or any(
|
||||||
|
part in cls._SENSITIVE_NAMES for part in lowered.split("_")
|
||||||
|
)
|
||||||
|
|
||||||
RESTRICTED: dict[str, dict[str, Any]] = {
|
RESTRICTED: dict[str, dict[str, Any]] = {
|
||||||
"max_iterations": {"type": int, "min": 1, "max": 100},
|
"max_iterations": {"type": int, "min": 1, "max": 100},
|
||||||
"context_window_tokens": {"type": int, "min": 4096, "max": 1_000_000},
|
"context_window_tokens": {"type": int, "min": 4096, "max": 1_000_000},
|
||||||
@@ -248,13 +255,16 @@ class MyTool(Tool):
|
|||||||
return f"{key}: {r}" if key else r
|
return f"{key}: {r}" if key else r
|
||||||
# Complex object — small Pydantic models: show values; others: show field names for navigation
|
# Complex object — small Pydantic models: show values; others: show field names for navigation
|
||||||
cls_name = type(val).__name__
|
cls_name = type(val).__name__
|
||||||
if hasattr(val, "model_fields"):
|
model_fields = getattr(type(val), "model_fields", None)
|
||||||
fields = list(val.model_fields.keys())
|
if model_fields:
|
||||||
|
fields = list(model_fields.keys())
|
||||||
if len(fields) <= 8:
|
if len(fields) <= 8:
|
||||||
# Small config objects: show field=value pairs
|
# Small config objects: show field=value pairs
|
||||||
pairs = []
|
pairs = []
|
||||||
for f in fields:
|
for f in fields:
|
||||||
fv = getattr(val, f, "?")
|
fv = getattr(val, f, "?")
|
||||||
|
if MyTool._is_sensitive_field_name(f):
|
||||||
|
continue
|
||||||
if isinstance(fv, (str, int, float, bool, type(None))):
|
if isinstance(fv, (str, int, float, bool, type(None))):
|
||||||
pairs.append(f"{f}={fv!r}")
|
pairs.append(f"{f}={fv!r}")
|
||||||
else:
|
else:
|
||||||
|
|||||||
@@ -7,6 +7,7 @@ from pathlib import Path
|
|||||||
from unittest.mock import AsyncMock, MagicMock
|
from unittest.mock import AsyncMock, MagicMock
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
from pydantic import BaseModel
|
||||||
|
|
||||||
from nanobot.agent.tools.self import MyTool
|
from nanobot.agent.tools.self import MyTool
|
||||||
|
|
||||||
@@ -168,6 +169,25 @@ class TestInspectPathNavigation:
|
|||||||
result = await tool.execute(action="check", key="tools")
|
result = await tool.execute(action="check", key="tools")
|
||||||
assert "not accessible" in result
|
assert "not accessible" in result
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_inspect_nested_config_redacts_sensitive_scalar_fields(self):
|
||||||
|
class SearchConfig(BaseModel):
|
||||||
|
provider: str = "tavily"
|
||||||
|
api_key: str = "sk-test-secret"
|
||||||
|
base_url: str = ""
|
||||||
|
max_results: int = 5
|
||||||
|
|
||||||
|
loop = _make_mock_loop()
|
||||||
|
loop.web_config = MagicMock()
|
||||||
|
loop.web_config.search = SearchConfig()
|
||||||
|
tool = _make_tool(loop)
|
||||||
|
|
||||||
|
result = await tool.execute(action="check", key="web_config.search")
|
||||||
|
|
||||||
|
assert "provider='tavily'" in result
|
||||||
|
assert "sk-test-secret" not in result
|
||||||
|
assert "api_key" not in result.lower()
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|||||||
Reference in New Issue
Block a user