mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(workspace): A4 downgrade expected refresh-fetch auth-fail to DEBUG
Smoke run 3 fired the same workspace.py warning ~9x per run: 'ensure_workspace: refresh fetch returned non-zero' stderr: 'fatal: could not read Username for https://github.com' This is EXPECTED behavior, not a bug. The docstring on _fetch_origin_best_effort explains that credentials are deliberately scrubbed from .git/config after the initial clone (part of the secret- exfiltration mitigation) and refresh fetches are best-effort. For private repos the auth-fail is the documented outcome. The original A4 spec proposed re-injecting the PAT -- that would have violated _assert_no_pat_leak and the URL-scrub mitigation. Re-scoped to: silence the known-benign signature at DEBUG, keep WARNING for genuine failures (network errors, broken remotes, repo-not-found). No behavior change. No security boundary touched. Just log level. Spec ref: docs/superpowers/specs/2026-05-12-post-smoke-3-fixes-design.md A4 (re-scoped 2026-05-12 after investigation showed the original spec proposed reintroducing a documented security regression).
This commit is contained in:
@@ -350,11 +350,24 @@ class WorkspaceService:
|
|||||||
return
|
return
|
||||||
|
|
||||||
if result.returncode != 0:
|
if result.returncode != 0:
|
||||||
logger.warning(
|
stderr = result.stderr.strip()
|
||||||
|
# The credential-less refresh fetch is expected to fail for private
|
||||||
|
# repos — see this method's docstring. Downgrade the known-benign
|
||||||
|
# auth-failure signature to DEBUG so it doesn't pollute every
|
||||||
|
# monitor / log scrape during a smoke run. Genuine failures
|
||||||
|
# (network errors, broken remotes) still surface at WARNING.
|
||||||
|
is_expected_auth_fail = (
|
||||||
|
"could not read Username" in stderr
|
||||||
|
or "Authentication failed" in stderr
|
||||||
|
or "remote: Repository not found" in stderr
|
||||||
|
)
|
||||||
|
log = logger.debug if is_expected_auth_fail else logger.warning
|
||||||
|
log(
|
||||||
"ensure_workspace: refresh fetch returned non-zero",
|
"ensure_workspace: refresh fetch returned non-zero",
|
||||||
workspace=str(workspace),
|
workspace=str(workspace),
|
||||||
project=project_slug,
|
project=project_slug,
|
||||||
stderr=result.stderr.strip(),
|
stderr=stderr,
|
||||||
|
expected_auth_fail=is_expected_auth_fail,
|
||||||
)
|
)
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
|
|||||||
@@ -0,0 +1,94 @@
|
|||||||
|
"""Wave A4 (2026-05-12): credentials-stripped refresh fetch logs at DEBUG,
|
||||||
|
not WARNING, when stderr is the known-benign auth-failure signature.
|
||||||
|
|
||||||
|
The credential-less fetch is intentional (workspace.py:317-323 explains).
|
||||||
|
Logging the expected auth-fail at WARNING level pollutes every monitor.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from pathlib import Path
|
||||||
|
from subprocess import CompletedProcess
|
||||||
|
from unittest.mock import patch
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
from roboco.services.workspace import (
|
||||||
|
WorkspaceService,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_fetch_auth_fail_logs_at_debug() -> None:
|
||||||
|
"""`fatal: could not read Username` stderr → DEBUG log, not WARNING."""
|
||||||
|
workspace = Path("/tmp/fake-workspace")
|
||||||
|
fake_result = CompletedProcess(
|
||||||
|
args=["git", "fetch", "origin"],
|
||||||
|
returncode=128,
|
||||||
|
stdout="",
|
||||||
|
stderr=(
|
||||||
|
"fatal: could not read Username for 'https://github.com': "
|
||||||
|
"terminal prompts disabled\n"
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
captured: list[tuple[str, str]] = []
|
||||||
|
|
||||||
|
def capture_warning(event: str, **_kw: object) -> None:
|
||||||
|
captured.append(("warning", event))
|
||||||
|
|
||||||
|
def capture_debug(event: str, **_kw: object) -> None:
|
||||||
|
captured.append(("debug", event))
|
||||||
|
|
||||||
|
with patch(
|
||||||
|
"roboco.services.workspace.subprocess.run", return_value=fake_result
|
||||||
|
), patch(
|
||||||
|
"roboco.services.workspace.logger.warning", side_effect=capture_warning
|
||||||
|
), patch(
|
||||||
|
"roboco.services.workspace.logger.debug", side_effect=capture_debug
|
||||||
|
):
|
||||||
|
await WorkspaceService._fetch_origin_best_effort(
|
||||||
|
workspace=workspace, project_slug="roboco-api"
|
||||||
|
)
|
||||||
|
|
||||||
|
# The benign auth-fail should NOT be a WARNING.
|
||||||
|
warnings = [e for (level, e) in captured if level == "warning"]
|
||||||
|
debugs = [e for (level, e) in captured if level == "debug"]
|
||||||
|
assert not warnings, f"expected no WARNING, got: {warnings}"
|
||||||
|
assert debugs, "expected at least one DEBUG entry, got nothing"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_fetch_genuine_failure_still_warns() -> None:
|
||||||
|
"""Network errors / other real failures must still log at WARNING."""
|
||||||
|
workspace = Path("/tmp/fake-workspace")
|
||||||
|
fake_result = CompletedProcess(
|
||||||
|
args=["git", "fetch", "origin"],
|
||||||
|
returncode=128,
|
||||||
|
stdout="",
|
||||||
|
stderr=(
|
||||||
|
"fatal: unable to access 'https://github.com/owner/repo.git/': "
|
||||||
|
"Could not resolve host: github.com\n"
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
captured: list[tuple[str, str]] = []
|
||||||
|
|
||||||
|
def capture_warning(event: str, **_kw: object) -> None:
|
||||||
|
captured.append(("warning", event))
|
||||||
|
|
||||||
|
def capture_debug(event: str, **_kw: object) -> None:
|
||||||
|
captured.append(("debug", event))
|
||||||
|
|
||||||
|
with patch(
|
||||||
|
"roboco.services.workspace.subprocess.run", return_value=fake_result
|
||||||
|
), patch(
|
||||||
|
"roboco.services.workspace.logger.warning", side_effect=capture_warning
|
||||||
|
), patch(
|
||||||
|
"roboco.services.workspace.logger.debug", side_effect=capture_debug
|
||||||
|
):
|
||||||
|
await WorkspaceService._fetch_origin_best_effort(
|
||||||
|
workspace=workspace, project_slug="roboco-api"
|
||||||
|
)
|
||||||
|
|
||||||
|
warnings = [e for (level, e) in captured if level == "warning"]
|
||||||
|
assert warnings, f"network errors should still warn, got: {captured}"
|
||||||
Reference in New Issue
Block a user