Use SecretStr for sensitive fields in configuration and GitHubPoller, removing custom masking logic. Update tests accordingly.

Author: coordt

foreman/__main__.py

@@ -116,7 +116,7 @@ def _run_start(args: Any) -> None:
     memory = MemoryStore(db_path)
 
     # 3. Create core components.
-    poller = GitHubPoller(token=str(config.identity.github_token), memory=memory)
+    poller = GitHubPoller(token=config.identity.github_token, memory=memory)
     dispatcher = Dispatcher(config=config, memory=memory)
 
     logger.info(

foreman/config.py

@@ -12,7 +12,7 @@
 from typing import Any, Optional
 
 import yaml
-from pydantic import BaseModel, field_validator, model_validator
+from pydantic import BaseModel, SecretStr, model_validator
 
 _ENV_REF_RE = re.compile(r"\$\{([^}]+)\}")
 
@@ -67,23 +67,6 @@ def _resolve_refs_in(obj: Any) -> Any:
     return obj
 
 
-# ---------------------------------------------------------------------------
-# Secret-masking string type
-# ---------------------------------------------------------------------------
-
-
-class _SecretStr(str):
-    """A string subclass that masks its value in repr and str output."""
-
-    def __repr__(self) -> str:
-        """Return masked representation."""
-        return "'**********'"
-
-    def __str__(self) -> str:
-        """Return masked string value."""
-        return "**********"
-
-
 # ---------------------------------------------------------------------------
 # Pydantic models
 # ---------------------------------------------------------------------------
@@ -92,18 +75,12 @@ def __str__(self) -> str:
 class IdentityConfig(BaseModel):
     """Bot GitHub identity configuration."""
 
-    github_token: str
+    github_token: SecretStr
     """GitHub Personal Access Token for the bot account."""
 
     github_user: str
     """GitHub username of the bot account."""
 
-    @field_validator("github_token", mode="after")
-    @classmethod
-    def mask_token(cls, v: str) -> str:
-        """Wrap the token in a secret-masking string."""
-        return _SecretStr(v)
-
 
 class LLMConfig(BaseModel):
     """LLM backend configuration."""
@@ -114,15 +91,9 @@ class LLMConfig(BaseModel):
     model: str
     """Model name / identifier."""
 
-    api_key: Optional[str] = None
+    api_key: Optional[SecretStr] = None
     """API key; omit for local providers such as Ollama."""
 
-    @field_validator("api_key", mode="after")
-    @classmethod
-    def mask_api_key(cls, v: Optional[str]) -> Optional[str]:
-        """Wrap the API key in a secret-masking string if present."""
-        return _SecretStr(v) if v is not None else None
-
 
 class PollingConfig(BaseModel):
     """GitHub polling configuration."""

foreman/poller.py

@@ -1,6 +1,6 @@
 """Asyncio polling loop for GitHub repositories.
 
-Polls all configured repositories concurrently, bounded by a semaphore to
+Polls all configured repositories concurrently, bounded by semaphore to
 avoid GitHub API rate limits.  Issues authored by repo collaborators are
 skipped by default.  Exponential backoff is applied on 403/429 responses.
 """
@@ -12,21 +12,23 @@
 from typing import TYPE_CHECKING, Any
 
 import structlog
-from github import Github, GithubException
+from github import Auth, Github, GithubException
 
 if TYPE_CHECKING:
     from collections.abc import Awaitable, Callable
 
+    from pydantic import SecretStr
+
     from foreman.config import RepoConfig
     from foreman.memory import MemoryStore
 
 logger = structlog.get_logger(__name__)
 
-#: Default maximum number of repos polled at the same time.
 _DEFAULT_MAX_CONCURRENT = 5
+"""Default maximum number of repos polled at the same time."""
 
-#: Initial backoff delay in seconds on rate-limit errors.
 _BACKOFF_BASE_SECONDS = 10.0
+"""Initial backoff delay in seconds on rate-limit errors."""
 
 
 class GitHubPoller:
@@ -42,8 +44,9 @@ class GitHubPoller:
         max_concurrent: Maximum number of repos polled simultaneously.
     """
 
-    def __init__(self, token: str, memory: MemoryStore, max_concurrent: int = _DEFAULT_MAX_CONCURRENT) -> None:
-        self._github = Github(token)
+    def __init__(self, token: SecretStr, memory: MemoryStore, max_concurrent: int = _DEFAULT_MAX_CONCURRENT) -> None:
+        auth = Auth.Token(token.get_secret_value())
+        self._github = Github(auth=auth)
         self._memory = memory
         self._max_concurrent = max_concurrent
 

tests/test_config.py

@@ -80,7 +80,7 @@ def test_returns_correct_identity(self, valid_config_file: Path) -> None:
         """Loaded config contains expected identity values."""
         config = load_config(valid_config_file)
         assert config.identity.github_user == "test-bot"
-        assert config.identity.github_token == "ghp_test_token"
+        assert config.identity.github_token.get_secret_value() == "ghp_test_token"
 
     def test_returns_correct_llm_config(self, valid_config_file: Path) -> None:
         """Loaded config contains expected LLM values."""
@@ -138,8 +138,8 @@ def test_env_refs_resolved_from_environment(
         monkeypatch.setenv("GITHUB_TOKEN", "ghp_from_env")
         monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-from-env")
         config = load_config(env_ref_config_file)
-        assert config.identity.github_token == "ghp_from_env"
-        assert config.llm.api_key == "sk-from-env"
+        assert config.identity.github_token.get_secret_value() == "ghp_from_env"
+        assert config.llm.api_key.get_secret_value() == "sk-from-env"
 
     def test_missing_env_var_raises_config_error(
         self, env_ref_config_file: Path, monkeypatch: pytest.MonkeyPatch

tests/test_poller.py

@@ -7,6 +7,7 @@
 from unittest.mock import AsyncMock, MagicMock
 
 import pytest
+from pydantic import SecretStr
 
 from foreman.config import RepoConfig
 from foreman.memory import MemoryStore
@@ -75,7 +76,7 @@ def test_returns_events_for_open_issues(self, memory: MemoryStore, mocker) -> No
         mock_repo.get_issues.return_value = [make_issue(1), make_issue(2)]
         mock_repo.get_collaborators.return_value = []
 
-        poller = GitHubPoller(token="test-token", memory=memory)
+        poller = GitHubPoller(token=SecretStr("test-token"), memory=memory)
         events = poller.poll_repo(RepoConfig(owner="owner", name="repo"))
 
         assert len(events) == 2
@@ -88,7 +89,7 @@ def test_event_has_required_fields(self, memory: MemoryStore, mocker) -> None:
         mock_repo.get_issues.return_value = [make_issue(7)]
         mock_repo.get_collaborators.return_value = []
 
-        poller = GitHubPoller(token="test-token", memory=memory)
+        poller = GitHubPoller(token=SecretStr("test-token"), memory=memory)
         events = poller.poll_repo(RepoConfig(owner="owner", name="repo"))
 
         assert len(events) == 1
@@ -108,7 +109,7 @@ def test_skips_issues_by_collaborators(self, memory: MemoryStore, mocker) -> Non
         ]
         mock_repo.get_collaborators.return_value = [make_collaborator("maintainer")]
 
-        poller = GitHubPoller(token="test-token", memory=memory)
+        poller = GitHubPoller(token=SecretStr("test-token"), memory=memory)
         events = poller.poll_repo(RepoConfig(owner="owner", name="repo"))
 
         assert len(events) == 1
@@ -125,7 +126,7 @@ def test_passes_since_to_get_issues_when_last_polled_set(self, memory: MemorySto
         last_polled = datetime(2024, 6, 1, tzinfo=timezone.utc)
         memory.set_last_polled("owner/repo", last_polled)
 
-        poller = GitHubPoller(token="test-token", memory=memory)
+        poller = GitHubPoller(token=SecretStr("test-token"), memory=memory)
         poller.poll_repo(RepoConfig(owner="owner", name="repo"))
 
         call_kwargs = mock_repo.get_issues.call_args
@@ -139,7 +140,7 @@ def test_get_issues_called_without_since_on_first_poll(self, memory: MemoryStore
         mock_repo.get_issues.return_value = []
         mock_repo.get_collaborators.return_value = []
 
-        poller = GitHubPoller(token="test-token", memory=memory)
+        poller = GitHubPoller(token=SecretStr("test-token"), memory=memory)
         poller.poll_repo(RepoConfig(owner="owner", name="repo"))
 
         call_kwargs = mock_repo.get_issues.call_args
@@ -155,7 +156,7 @@ def test_updates_last_polled_after_successful_poll(self, memory: MemoryStore, mo
 
         assert memory.get_last_polled("owner/repo") is None
 
-        poller = GitHubPoller(token="test-token", memory=memory)
+        poller = GitHubPoller(token=SecretStr("test-token"), memory=memory)
         poller.poll_repo(RepoConfig(owner="owner", name="repo"))
 
         assert memory.get_last_polled("owner/repo") is not None
@@ -168,11 +169,11 @@ def test_last_polled_survives_new_instance(self, memory: MemoryStore, mocker) ->
         mock_repo.get_issues.return_value = []
         mock_repo.get_collaborators.return_value = []
 
-        poller1 = GitHubPoller(token="test-token", memory=memory)
+        poller1 = GitHubPoller(token=SecretStr("test-token"), memory=memory)
         poller1.poll_repo(RepoConfig(owner="owner", name="repo"))
         ts1 = memory.get_last_polled("owner/repo")
 
-        poller2 = GitHubPoller(token="test-token", memory=memory)
+        poller2 = GitHubPoller(token=SecretStr("test-token"), memory=memory)
         poller2.poll_repo(RepoConfig(owner="owner", name="repo"))
         ts2 = memory.get_last_polled("owner/repo")
 
@@ -188,7 +189,7 @@ def test_payload_contains_issue_metadata(self, memory: MemoryStore, mocker) -> N
         mock_repo.get_issues.return_value = [make_issue(3, author_login="alice", title="A bug")]
         mock_repo.get_collaborators.return_value = []
 
-        poller = GitHubPoller(token="test-token", memory=memory)
+        poller = GitHubPoller(token=SecretStr("test-token"), memory=memory)
         events = poller.poll_repo(RepoConfig(owner="owner", name="repo"))
 
         payload = events[0]["payload"]
@@ -211,7 +212,7 @@ async def test_poll_all_calls_poll_repo_for_each_repo(self, memory: MemoryStore,
         mocker.patch("foreman.poller.Github")
         mock_poll = mocker.patch.object(GitHubPoller, "poll_repo", return_value=[])
 
-        poller = GitHubPoller(token="test-token", memory=memory)
+        poller = GitHubPoller(token=SecretStr("test-token"), memory=memory)
         repos = [
             RepoConfig(owner="org", name="repo1"),
             RepoConfig(owner="org", name="repo2"),
@@ -235,7 +236,7 @@ def side_effect(repo_cfg: RepoConfig):
 
         mocker.patch.object(GitHubPoller, "poll_repo", side_effect=side_effect)
 
-        poller = GitHubPoller(token="test-token", memory=memory)
+        poller = GitHubPoller(token=SecretStr("test-token"), memory=memory)
         repos = [RepoConfig(owner="org", name="repo1"), RepoConfig(owner="org", name="repo2")]
         collected: list[dict] = []
 
@@ -250,7 +251,7 @@ async def callback(repo_cfg: RepoConfig, event: dict) -> None:
     async def test_poll_all_with_empty_repo_list(self, memory: MemoryStore, mocker) -> None:
         """poll_all completes without error when given an empty repo list."""
         mocker.patch("foreman.poller.Github")
-        poller = GitHubPoller(token="test-token", memory=memory)
+        poller = GitHubPoller(token=SecretStr("test-token"), memory=memory)
         await poller.poll_all([], AsyncMock())  # must not raise
 
 
@@ -281,7 +282,7 @@ def fail_then_succeed(repo_cfg: RepoConfig):
 
         mocker.patch.object(GitHubPoller, "poll_repo", side_effect=fail_then_succeed)
 
-        poller = GitHubPoller(token="test-token", memory=memory)
+        poller = GitHubPoller(token=SecretStr("test-token"), memory=memory)
         await poller.poll_all([RepoConfig(owner="owner", name="repo")], AsyncMock())
 
         assert call_count == 2, "should retry once after 429"
@@ -306,7 +307,7 @@ def fail_then_succeed(repo_cfg: RepoConfig):
 
         mocker.patch.object(GitHubPoller, "poll_repo", side_effect=fail_then_succeed)
 
-        poller = GitHubPoller(token="test-token", memory=memory)
+        poller = GitHubPoller(token=SecretStr("test-token"), memory=memory)
         await poller.poll_all([RepoConfig(owner="owner", name="repo")], AsyncMock())
 
         assert call_count == 2
@@ -325,7 +326,7 @@ async def test_poll_all_skips_repo_after_repeated_failures(self, memory: MemoryS
             side_effect=GithubException(429, {"message": "rate limited"}, {}),
         )
 
-        poller = GitHubPoller(token="test-token", memory=memory)
+        poller = GitHubPoller(token=SecretStr("test-token"), memory=memory)
         # Must not raise
         await poller.poll_all([RepoConfig(owner="owner", name="repo")], AsyncMock())
 
@@ -345,7 +346,7 @@ async def test_non_rate_limit_github_exception_logs_error_and_skips_repo(
             side_effect=GithubException(500, {"message": "server error"}, {}),
         )
 
-        poller = GitHubPoller(token="test-token", memory=memory)
+        poller = GitHubPoller(token=SecretStr("test-token"), memory=memory)
         # Must not raise — error is logged and repo is skipped this cycle
         await poller.poll_all([RepoConfig(owner="owner", name="repo")], AsyncMock())
 
@@ -365,7 +366,7 @@ async def test_bad_credentials_logs_critical_and_skips_repo(self, memory: Memory
             side_effect=GithubException(401, {"message": "Bad credentials"}, {}),
         )
 
-        poller = GitHubPoller(token="test-token", memory=memory)
+        poller = GitHubPoller(token=SecretStr("test-token"), memory=memory)
         # Must not raise — critical is logged and repo is skipped
         await poller.poll_all([RepoConfig(owner="owner", name="repo")], AsyncMock())
 
@@ -383,14 +384,14 @@ class TestSemaphore:
     def test_default_max_concurrent_is_five(self, memory: MemoryStore, mocker) -> None:
         """GitHubPoller defaults to max_concurrent=5."""
         mocker.patch("foreman.poller.Github")
-        poller = GitHubPoller(token="test-token", memory=memory)
-        assert poller._max_concurrent == 5  # noqa: SLF001
+        poller = GitHubPoller(token=SecretStr("test-token"), memory=memory)
+        assert poller._max_concurrent == 5
 
     def test_custom_max_concurrent_is_stored(self, memory: MemoryStore, mocker) -> None:
         """max_concurrent passed to __init__ is stored on the instance."""
         mocker.patch("foreman.poller.Github")
-        poller = GitHubPoller(token="test-token", memory=memory, max_concurrent=2)
-        assert poller._max_concurrent == 2  # noqa: SLF001
+        poller = GitHubPoller(token=SecretStr("test-token"), memory=memory, max_concurrent=2)
+        assert poller._max_concurrent == 2
 
 
 # ---------------------------------------------------------------------------