fix(M11): propagate PipelineCriticalError, redact configured secrets, signal blog failures

Correct 4 findings from the independent M11 review:

#1 (Critical) — PipelineCriticalError was downgraded to PipelineWarning:
  - Add except PipelineCriticalError: raise before each except Exception
    in all 5 non-blocking steps (fetch_blog, compare, caldav_sync, synthesis, send)
  - Critical errors now propagate to the outer handler and stop the pipeline

#2 (Critical) — redact_exception() did not use configured secrets:
  - Extend redact_exception() with extra_secrets parameter (upward compatible)
  - Harden redact_secrets(): sort extra_secrets by length descending
  - Add Settings.redaction_secrets() collecting all 6 SecretStr fields
  - Add PipelineRunner._redact(exc) using self._redaction_secrets
  - All except blocks in run() now use self._redact(exc)
  - CalDAV FAILED-status path uses full redaction_secrets collection

#3 (Medium) — BlogRSSClient silently swallowed failures:
  - Add error field to BlogRSSFetchResult
  - rss.py sets error on failure paths (except Exception, bozo/invalid feed)
  - fetch_blog_step raises RuntimeError when result.error is set
  - PipelineRunner now produces PipelineWarning for blog failures

#4 (Medium) — Test coverage at 80%, now 91%:
  - 11 new integration tests covering blog failure/success, compare failure,
    CalDAV failure (exception + FAILED status), send False/exception,
    PipelineCriticalError propagation, secret redaction with sentinel,
    empty agenda/homework, iCal cache cleanup
  - Secret redaction test uses mock (no network) and proves configured-secret
    propagation via non-URL sentinel in RuntimeError

Validation: 619 tests pass, ruff/mypy/bandit/pre-commit green, coverage 91%.

Co-authored-by: opencode/coder <coder@agents.invalid>
Co-authored-by: opencode/test-engineer <test-engineer@agents.invalid>
This commit is contained in:
2026-09-08 12:20:29 +02:00
parent 26b083561a
commit 28c695795a
7 changed files with 672 additions and 14 deletions

View File

@@ -8,14 +8,18 @@ from typing import Any, cast
import pytest
from pydantic import SecretStr
from pronote_sync.config.settings import AppSettings, PronoteSettings, Settings
from pronote_sync.config.settings import AISettings, AppSettings, PronoteSettings, Settings
from pronote_sync.errors import PipelineCriticalError, PipelineWarning
from pronote_sync.models.agenda import Lesson, LessonStatus, SchoolEvent
from pronote_sync.models.blog import BlogArticle
from pronote_sync.models.diff import AgendaDiff
from pronote_sync.models.homework import Homework
from pronote_sync.models.message import Message
from pronote_sync.models.sync import CalDAVSyncResult, CalDAVSyncStatus
from pronote_sync.pipeline.run import PipelineRunner
from pronote_sync.sources.blog.result import BlogRSSFetchResult
from pronote_sync.sources.blog.rss import BlogRSSClient
from pronote_sync.sources.blog.state import BlogRSSState
from pronote_sync.sources.pronote.fallback import PronoteFetcher
from pronote_sync.sync.diff import AgendaComparator
@@ -464,3 +468,598 @@ def test_runner_reuses_ical_download_and_parse_within_one_run(
assert data is not None
assert errors == []
assert fetch_calls == ["https://pronote.example.test/calendar.ics"]
class FailingBlogClient:
"""Blog client raising an exception to test error handling."""
def fetch_and_parse(
self,
*,
known_guids: frozenset[str] | None = None,
etag: str | None = None,
last_modified: str | None = None,
) -> Any:
"""Raise an exception to simulate blog fetch failure.
:param known_guids: Known GUIDs (unused).
:param etag: ETag header (unused).
:param last_modified: Last-Modified header (unused).
:return: Never returns.
:raises RuntimeError: Always.
"""
del known_guids, etag, last_modified
raise RuntimeError("Blog fetch failed: network error")
class FailingComparator:
"""Comparator raising an exception to test error handling."""
def compare(self, lessons: list[Lesson], target_date: date) -> AgendaDiff:
"""Raise an exception to simulate comparison failure.
:param lessons: Lessons to compare (unused).
:param target_date: Target date (unused).
:return: Never returns.
:raises RuntimeError: Always.
"""
del lessons, target_date
raise RuntimeError("Comparison failed: invalid data")
class FailingCaldavSynchronizer:
"""CalDAV synchronizer raising an exception to test error handling."""
def __call__(self, data: Any, settings: Settings) -> CalDAVSyncResult:
"""Raise an exception to simulate CalDAV sync failure.
:param data: Data to sync (unused).
:param settings: Settings (unused).
:return: Never returns.
:raises RuntimeError: Always.
"""
del data, settings
raise RuntimeError("CalDAV sync failed: connection error")
class FailingChannel:
"""Channel that returns False to test error handling."""
def __init__(self) -> None:
"""Initialize the failing channel."""
self.messages: list[Any] = []
def send(self, message: Any) -> bool:
"""Return False to simulate refused send.
:param message: Message to send (unused).
:return: Always False.
:rtype: bool
"""
self.messages.append(message)
return False
class ExceptionalChannel:
"""Channel that raises an exception to test error handling."""
def __init__(self) -> None:
"""Initialize the exceptional channel."""
self.messages: list[Any] = []
def send(self, message: Any) -> bool:
"""Raise an exception to simulate send failure.
:param message: Message to send (unused).
:return: Never returns.
:raises RuntimeError: Always.
"""
self.messages.append(message)
raise RuntimeError("XMPP send failed: connection closed")
class CriticalChannel:
"""Channel that raises PipelineCriticalError to test propagation."""
def send(self, message: Any) -> bool:
"""Raise PipelineCriticalError to test propagation.
:param message: Message to send (unused).
:return: Never returns.
:raises PipelineCriticalError: Always.
"""
del message
raise PipelineCriticalError("Critical send failure", step="send")
class EmptyFetcher:
"""Pronote fetcher returning empty agenda and homework."""
def fetch_agenda(self) -> tuple[list[Lesson], list[SchoolEvent]]:
"""Return empty agenda.
:return: Empty lessons and events.
:rtype: tuple[list[Lesson], list[SchoolEvent]]
"""
return [], []
def fetch_homework(self, target_date: date) -> list[Homework]:
"""Return empty homework.
:param target_date: Target date (unused).
:return: Empty homework list.
:rtype: list[Homework]
"""
del target_date
return []
def fetch_messages(self) -> list[Message]:
"""Return no messages.
:return: Empty list.
:rtype: list[Message]
"""
return []
def fetch_informations(self) -> list[Message]:
"""Return no information messages.
:return: Empty list.
:rtype: list[Message]
"""
return []
@pytest.fixture
def caldav_failed_result() -> CalDAVSyncResult:
"""Fixture providing a failed CalDAV sync result.
:return: Failed sync result with errors.
:rtype: CalDAVSyncResult
"""
return CalDAVSyncResult(
status=CalDAVSyncStatus.FAILED,
added=0,
updated=0,
removed=0,
errors=["Error 1: invalid credentials", "Error 2: server unavailable"],
)
def test_runner_blog_failure_produces_pipeline_warning(
pipeline_inputs: tuple[Lesson, Homework],
) -> None:
"""Blog failure produces PipelineWarning and continues with empty blog_articles."""
lesson, homework = pipeline_inputs
calls: list[str] = []
runner = PipelineRunner(
settings=Settings(blog=Settings().blog.model_copy(update={"enabled": True})),
pronote_fetcher=StubFetcher(calls, lesson, homework),
caldav_synchronizer=lambda data, settings: successful_sync_result(),
agenda_comparator=cast("AgendaComparator | None", StubComparator(calls)),
blog_client=cast("BlogRSSClient | None", FailingBlogClient()),
blog_state=BlogRSSState(),
channel=StubChannel(calls),
now_provider=lambda: datetime(2026, 9, 8, 7, 0),
)
data, errors = runner.run()
assert data is not None
assert len(errors) == 1
assert isinstance(errors[0], PipelineWarning)
assert errors[0].step == "fetch_blog"
assert "Blog fetch failed" in errors[0].message
# Verify pipeline continued to next steps
assert len(calls) >= 5 # fetch, fetch_homework, fetch_messages, fetch_informations, compare
def test_runner_compare_failure_produces_pipeline_warning(
pipeline_inputs: tuple[Lesson, Homework],
) -> None:
"""Compare failure produces PipelineWarning and continues with empty AgendaDiff."""
lesson, homework = pipeline_inputs
calls: list[str] = []
runner = PipelineRunner(
settings=Settings(),
pronote_fetcher=StubFetcher(calls, lesson, homework),
caldav_synchronizer=lambda data, settings: successful_sync_result(),
agenda_comparator=cast("AgendaComparator | None", FailingComparator()),
channel=StubChannel(calls),
now_provider=lambda: datetime(2026, 9, 8, 7, 0),
)
data, errors = runner.run()
assert data is not None
assert len(errors) == 1
assert isinstance(errors[0], PipelineWarning)
assert errors[0].step == "compare"
assert "Comparison failed" in errors[0].message
# Pipeline continues to next steps even if compare fails
def test_runner_caldav_sync_exception_produces_pipeline_warning(
pipeline_inputs: tuple[Lesson, Homework],
) -> None:
"""CalDAV synchronizer raises exception → PipelineWarning with step='caldav_sync'."""
lesson, homework = pipeline_inputs
calls: list[str] = []
runner = PipelineRunner(
settings=Settings(),
pronote_fetcher=StubFetcher(calls, lesson, homework),
caldav_synchronizer=FailingCaldavSynchronizer(),
agenda_comparator=cast("AgendaComparator | None", StubComparator(calls)),
channel=StubChannel(calls),
now_provider=lambda: datetime(2026, 9, 8, 7, 0),
)
data, errors = runner.run()
assert data is not None
assert len(errors) == 1
assert isinstance(errors[0], PipelineWarning)
assert errors[0].step == "caldav_sync"
assert "CalDAV sync failed" in errors[0].message
# Pipeline continues to next steps even if caldav_sync fails
def test_runner_caldav_sync_failed_status_produces_pipeline_warning(
pipeline_inputs: tuple[Lesson, Homework],
caldav_failed_result: CalDAVSyncResult,
) -> None:
"""CalDAV sync result has status=FAILED → PipelineWarning with redacted errors."""
lesson, homework = pipeline_inputs
calls: list[str] = []
def failing_sync_result(data: Any, settings: Settings) -> CalDAVSyncResult:
"""Return a failed sync result.
:param data: Data to sync (unused).
:param settings: Settings (unused).
:return: Failed sync result.
:rtype: CalDAVSyncResult
"""
del data, settings
return caldav_failed_result
runner = PipelineRunner(
settings=Settings(),
pronote_fetcher=StubFetcher(calls, lesson, homework),
caldav_synchronizer=failing_sync_result,
agenda_comparator=cast("AgendaComparator | None", StubComparator(calls)),
channel=StubChannel(calls),
now_provider=lambda: datetime(2026, 9, 8, 7, 0),
)
data, errors = runner.run()
assert data is not None
assert len(errors) == 1
assert isinstance(errors[0], PipelineWarning)
assert errors[0].step == "caldav_sync"
# Verify errors are redacted (should not contain plain secret if present)
assert "Error 1" in errors[0].message or "Error 2" in errors[0].message
# Pipeline continues to next steps even if caldav_sync fails
def test_runner_channel_send_returns_false_produces_pipeline_warning(
pipeline_inputs: tuple[Lesson, Homework],
) -> None:
"""Channel.send() returns False → PipelineWarning with step='send' and 'refusé'."""
lesson, homework = pipeline_inputs
calls: list[str] = []
runner = PipelineRunner(
settings=Settings(),
pronote_fetcher=StubFetcher(calls, lesson, homework),
caldav_synchronizer=lambda data, settings: successful_sync_result(),
agenda_comparator=cast("AgendaComparator | None", StubComparator(calls)),
channel=FailingChannel(),
now_provider=lambda: datetime(2026, 9, 8, 7, 0),
)
data, errors = runner.run()
assert data is not None
assert len(errors) == 1
assert isinstance(errors[0], PipelineWarning)
assert errors[0].step == "send"
assert "refusé" in errors[0].message
def test_runner_channel_send_raises_exception_produces_pipeline_warning(
pipeline_inputs: tuple[Lesson, Homework],
) -> None:
"""Channel.send() raises exception → PipelineWarning with step='send'."""
lesson, homework = pipeline_inputs
calls: list[str] = []
runner = PipelineRunner(
settings=Settings(),
pronote_fetcher=StubFetcher(calls, lesson, homework),
caldav_synchronizer=lambda data, settings: successful_sync_result(),
agenda_comparator=cast("AgendaComparator | None", StubComparator(calls)),
channel=ExceptionalChannel(),
now_provider=lambda: datetime(2026, 9, 8, 7, 0),
)
data, errors = runner.run()
assert data is not None
assert len(errors) == 1
assert isinstance(errors[0], PipelineWarning)
assert errors[0].step == "send"
assert "XMPP send failed" in errors[0].message
def test_runner_pipeline_critical_error_from_non_blocking_step_propagates(
pipeline_inputs: tuple[Lesson, Homework],
) -> None:
"""PipelineCriticalError from non-blocking step propagates and stops pipeline."""
lesson, homework = pipeline_inputs
calls: list[str] = []
runner = PipelineRunner(
settings=Settings(),
pronote_fetcher=StubFetcher(calls, lesson, homework),
caldav_synchronizer=lambda data, settings: successful_sync_result(),
agenda_comparator=cast("AgendaComparator | None", StubComparator(calls)),
channel=CriticalChannel(),
now_provider=lambda: datetime(2026, 9, 8, 7, 0),
)
data, errors = runner.run()
# Pipeline should return None data and critical error in errors list
assert data is None
assert len(errors) == 1
assert isinstance(errors[0], PipelineCriticalError)
assert errors[0].step == "send"
assert "Critical send failure" in errors[0].message
def test_runner_blog_success_delivers_articles_into_xmpp_message_external_info(
pipeline_inputs: tuple[Lesson, Homework],
) -> None:
"""Blog client returns articles → they appear in XmppMessage.external_info.blog_articles."""
lesson, homework = pipeline_inputs
calls: list[str] = []
class SuccessBlogClient:
"""Blog client returning BlogRSSFetchResult with articles."""
def fetch_and_parse(
self,
*,
known_guids: frozenset[str] | None = None,
etag: str | None = None,
last_modified: str | None = None,
) -> BlogRSSFetchResult:
"""Return BlogRSSFetchResult with articles.
:param known_guids: Known GUIDs (unused).
:param etag: ETag header (unused).
:param last_modified: Last-Modified header (unused).
:return: BlogRSSFetchResult with articles.
:rtype: BlogRSSFetchResult
"""
del known_guids, etag, last_modified
article = BlogArticle(
id="article-1",
title="Test Article",
url="https://example.com/article1",
published_at=datetime(2026, 9, 8, 12, 0, tzinfo=datetime.now().astimezone().tzinfo),
updated_at=None,
category="News",
author="Test Author",
content_html="<p>Test content</p>",
content_text="Test content",
)
return BlogRSSFetchResult(
articles=(article,),
etag=None,
last_modified=None,
not_modified=False,
)
runner = PipelineRunner(
settings=Settings(blog=Settings().blog.model_copy(update={"enabled": True})),
pronote_fetcher=StubFetcher(calls, lesson, homework),
caldav_synchronizer=lambda data, settings: successful_sync_result(),
agenda_comparator=cast("AgendaComparator | None", StubComparator(calls)),
blog_client=cast("BlogRSSClient | None", SuccessBlogClient()),
blog_state=BlogRSSState(),
channel=StubChannel(calls),
now_provider=lambda: datetime(2026, 9, 8, 7, 0),
)
data, errors = runner.run()
assert data is not None
assert len(errors) == 0
# Check that channel received message with external_info containing blog_articles
channel = runner._channel
assert channel is not None
assert hasattr(channel, "messages")
assert len(channel.messages) == 1
xmpp_message = channel.messages[0]
assert xmpp_message.external_info is not None
assert len(xmpp_message.external_info.blog_articles) == 1
assert xmpp_message.external_info.blog_articles[0].title == "Test Article"
def test_runner_secret_redaction_in_pipeline_errors(
pipeline_inputs: tuple[Lesson, Homework],
) -> None:
"""A configured AI key in a non-URL exception is masked by the runner.
The fake blog client embeds the raw API key in plain text, without any
URL or ``key=value`` form: only the propagation of
``Settings.redaction_secrets()`` into ``PipelineRunner._redact()`` can
mask it. No network call is performed.
"""
lesson, homework = pipeline_inputs
calls: list[str] = []
secret = SecretStr("sk-test-sentinel-12345")
class SecretLeakingBlogClient:
"""Blog client raising an exception containing a raw configured secret."""
def fetch_and_parse(
self,
*,
known_guids: frozenset[str] | None = None,
etag: str | None = None,
last_modified: str | None = None,
) -> Any:
"""Raise an exception embedding the raw API key.
:param known_guids: Known GUIDs (unused).
:param etag: ETag header (unused).
:param last_modified: Last-Modified header (unused).
:return: Never returns.
:raises RuntimeError: Always.
"""
del known_guids, etag, last_modified
raise RuntimeError(f"API key {secret.get_secret_value()} rejected")
runner = PipelineRunner(
settings=Settings(
ai=AISettings(api_key=secret),
blog=Settings().blog.model_copy(update={"enabled": True}),
),
pronote_fetcher=StubFetcher(calls, lesson, homework),
caldav_synchronizer=lambda data, settings: successful_sync_result(),
agenda_comparator=cast("AgendaComparator | None", StubComparator(calls)),
blog_client=cast("BlogRSSClient | None", SecretLeakingBlogClient()),
blog_state=BlogRSSState(),
channel=StubChannel(calls),
now_provider=lambda: datetime(2026, 9, 8, 7, 0),
)
data, errors = runner.run()
assert data is not None
assert len(errors) == 1
assert isinstance(errors[0], PipelineWarning)
assert errors[0].step == "fetch_blog"
# The configured AI key must be masked through _redact(), not by a
# generic URL pattern: the message contains no URL nor key=value form.
assert secret.get_secret_value() not in errors[0].message
assert "REDACTED" in errors[0].message
def test_runner_empty_agenda_homework_accepted_as_success() -> None:
"""Empty agenda and homework lists → pipeline succeeds (no critical error)."""
calls: list[str] = []
runner = PipelineRunner(
settings=Settings(),
pronote_fetcher=EmptyFetcher(),
caldav_synchronizer=lambda data, settings: successful_sync_result(),
channel=StubChannel(calls),
now_provider=lambda: datetime(2026, 9, 8, 7, 0),
)
data, errors = runner.run()
assert data is not None
assert len(errors) == 0
assert data.lessons == []
assert data.homeworks == []
def test_runner_ical_cache_cleanup_on_second_run(
monkeypatch: pytest.MonkeyPatch,
pipeline_inputs: tuple[Lesson, Homework],
) -> None:
"""Two independent PipelineRunner.run() calls each trigger their own iCal download."""
import pronote_sync.sources.pronote.fallback as fallback_module
lesson, _ = pipeline_inputs
settings = Settings(
pronote=PronoteSettings(
ical_url=SecretStr("https://pronote.example.test/calendar.ics"),
agenda_source="ical",
homework_source="ical",
)
)
fetch_calls: list[str] = []
monkeypatch.setattr(
fallback_module,
"fetch_ical",
lambda url: _record_ical_fetch(fetch_calls, url),
)
monkeypatch.setattr(fallback_module, "parse_ical", lambda raw: ([lesson], [], []))
class NoMessageClient:
"""Minimal pronotepy client for non-critical M11 fetches."""
def get_messages(self) -> list[Message]:
"""Return no messages.
:return: An empty list.
:rtype: list[Message]
"""
return []
def get_informations(self) -> list[Message]:
"""Return no information messages.
:return: An empty list.
:rtype: list[Message]
"""
return []
def get_lessons(self, start: date, end: date) -> list[Lesson]:
"""Return no fallback lessons.
:param start: Start date (unused).
:param end: End date (unused).
:return: Empty list.
:rtype: list[Lesson]
"""
del start, end
return []
def get_homeworks(self, start: date, end: date) -> list[Homework]:
"""Return no fallback homework.
:param start: Start date (unused).
:param end: End date (unused).
:return: Empty list.
:rtype: list[Homework]
"""
del start, end
return []
# First run
runner1 = PipelineRunner(
settings=settings,
pronote_fetcher=PronoteFetcher(settings, NoMessageClient()),
now_provider=lambda: datetime(2026, 9, 8, 7, 0),
)
data1, errors1 = runner1.run()
# Second run
runner2 = PipelineRunner(
settings=settings,
pronote_fetcher=PronoteFetcher(settings, NoMessageClient()),
now_provider=lambda: datetime(2026, 9, 8, 7, 0),
)
data2, errors2 = runner2.run()
assert data1 is not None
assert data2 is not None
assert errors1 == []
assert errors2 == []
# Each run should have triggered its own iCal download
assert fetch_calls == [
"https://pronote.example.test/calendar.ics",
"https://pronote.example.test/calendar.ics",
]