From 01f88a0ecaa5e4d9d29bb8d72e90c44e9f8ef478 Mon Sep 17 00:00:00 2001 From: sylvain Date: Tue, 10 Mar 2026 19:00:57 +0100 Subject: [PATCH] fix(audit): correct findings from review round 1 - client: add raise_for_status() in get_latest_release() for non-404 errors (FINDING-001) - client: add timeout parameter (default 30s) passed to all session.get() calls (FINDING-004/SEC-002) - cli: replace return with sys.exit(1) in all except blocks (FINDING-002) - test_cli: remove duplicate test_exits_when_token_missing, assert GITEA_TOKEN in stderr (FINDING-006) - test_cli: update connection error tests to expect SystemExit(1) after exit code fix - test_cli: rework token masking test to inject token into exception message (FINDING-007) - test_client: add test_raises_on_server_error for HTTP 500 path (FINDING-001) - test_client: add tests for default and custom timeout values (FINDING-004) Co-Authored-By: Claude Sonnet 4.6 --- src/gitea_dashboard/cli.py | 6 ++-- src/gitea_dashboard/client.py | 16 +++++++--- tests/test_cli.py | 55 ++++++++++++++++++++++------------- tests/test_client.py | 24 +++++++++++++++ 4 files changed, 74 insertions(+), 27 deletions(-) diff --git a/src/gitea_dashboard/cli.py b/src/gitea_dashboard/cli.py index 9ffe9a5..1a5f145 100644 --- a/src/gitea_dashboard/cli.py +++ b/src/gitea_dashboard/cli.py @@ -41,18 +41,18 @@ def main() -> None: repos = collect_all(client) except requests.ConnectionError: console.print("[red]Erreur : connexion refusee. Verifiez l'URL et le serveur Gitea.[/red]") - return + sys.exit(1) except requests.Timeout: console.print( "[red]Erreur : delai d'attente depasse. Le serveur Gitea ne repond pas.[/red]" ) - return + sys.exit(1) except requests.RequestException as exc: # Ne jamais afficher le token dans les messages d'erreur msg = str(exc) if token in msg: msg = msg.replace(token, "***") console.print(f"[red]Erreur API : {msg}[/red]") - return + sys.exit(1) render_dashboard(repos) diff --git a/src/gitea_dashboard/client.py b/src/gitea_dashboard/client.py index 738d546..d40214c 100644 --- a/src/gitea_dashboard/client.py +++ b/src/gitea_dashboard/client.py @@ -14,9 +14,16 @@ class GiteaClient: _PAGE_LIMIT = 50 - def __init__(self, base_url: str, token: str) -> None: - """Initialise le client avec l'URL de base et le token API.""" + def __init__(self, base_url: str, token: str, timeout: int = 30) -> None: + """Initialise le client avec l'URL de base et le token API. + + Args: + base_url: URL de base de l'instance Gitea. + token: Token API pour l'authentification. + timeout: Delai maximum en secondes pour chaque requete (defaut: 30). + """ self.base_url = base_url.rstrip("/") + self.timeout = timeout self.session = requests.Session() self.session.headers["Authorization"] = f"token {token}" @@ -33,7 +40,7 @@ class GiteaClient: merged_params["limit"] = self._PAGE_LIMIT merged_params["page"] = page url = f"{self.base_url}{endpoint}" - resp = self.session.get(url, params=merged_params) + resp = self.session.get(url, params=merged_params, timeout=self.timeout) resp.raise_for_status() items = resp.json() all_items.extend(items) @@ -57,9 +64,10 @@ class GiteaClient: Gere HTTP 404 en retournant None. """ url = f"{self.base_url}/api/v1/repos/{owner}/{repo}/releases/latest" - resp = self.session.get(url) + resp = self.session.get(url, timeout=self.timeout) if resp.status_code == 404: return None + resp.raise_for_status() return resp.json() def get_milestones(self, owner: str, repo: str) -> list[dict]: diff --git a/tests/test_cli.py b/tests/test_cli.py index 31743fa..58bb341 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -46,17 +46,15 @@ class TestMainNominal: class TestMainMissingToken: """Test main() when GITEA_TOKEN is not set.""" - def test_exits_when_token_missing(self): - """main() exits with SystemExit when GITEA_TOKEN is absent.""" + def test_error_message_when_token_missing(self, capsys): + """main() exits with code 1 and prints message mentioning GITEA_TOKEN.""" with patch.dict("os.environ", {}, clear=True): - with pytest.raises(SystemExit): + with pytest.raises(SystemExit) as exc_info: main() - def test_error_message_when_token_missing(self, capsys): - """main() prints a clear error message about missing token.""" - with patch.dict("os.environ", {}, clear=True): - with pytest.raises(SystemExit): - main() + assert exc_info.value.code == 1 + captured = capsys.readouterr() + assert "GITEA_TOKEN" in captured.err class TestMainConnectionErrors: @@ -65,48 +63,65 @@ class TestMainConnectionErrors: @patch("gitea_dashboard.cli.collect_all") @patch("gitea_dashboard.cli.GiteaClient") def test_connection_error_handled(self, mock_client_cls, mock_collect): - """ConnectionError is caught and does not crash.""" + """ConnectionError is caught and exits with code 1.""" env = {"GITEA_TOKEN": "test-token"} mock_client_cls.return_value = MagicMock() mock_collect.side_effect = requests.ConnectionError("Connection refused") with patch.dict("os.environ", env, clear=True): - # Should not raise — error is handled gracefully - main() + with pytest.raises(SystemExit) as exc_info: + main() + + assert exc_info.value.code == 1 @patch("gitea_dashboard.cli.collect_all") @patch("gitea_dashboard.cli.GiteaClient") def test_timeout_error_handled(self, mock_client_cls, mock_collect): - """Timeout is caught and does not crash.""" + """Timeout is caught and exits with code 1.""" env = {"GITEA_TOKEN": "test-token"} mock_client_cls.return_value = MagicMock() mock_collect.side_effect = requests.Timeout("Request timed out") with patch.dict("os.environ", env, clear=True): - main() + with pytest.raises(SystemExit) as exc_info: + main() + + assert exc_info.value.code == 1 @patch("gitea_dashboard.cli.collect_all") @patch("gitea_dashboard.cli.GiteaClient") def test_request_exception_handled(self, mock_client_cls, mock_collect): - """Generic RequestException is caught and does not crash.""" + """Generic RequestException is caught and exits with code 1.""" env = {"GITEA_TOKEN": "test-token"} mock_client_cls.return_value = MagicMock() mock_collect.side_effect = requests.RequestException("Something went wrong") with patch.dict("os.environ", env, clear=True): - main() + with pytest.raises(SystemExit) as exc_info: + main() + + assert exc_info.value.code == 1 @patch("gitea_dashboard.cli.collect_all") @patch("gitea_dashboard.cli.GiteaClient") def test_token_not_in_error_output(self, mock_client_cls, mock_collect, capsys): - """Token must never appear in error messages.""" + """Token must never appear in error messages, even when present in the exception.""" env = {"GITEA_TOKEN": "super-secret-token-xyz"} mock_client_cls.return_value = MagicMock() - mock_collect.side_effect = requests.ConnectionError("Connection refused") + + # Build exception message that embeds the token value from env + # to simulate a real-world leak scenario + def make_exc(environ): + leaked = environ["GITEA_TOKEN"] + return requests.RequestException(f"HTTP Error: Authorization token {leaked} rejected") + + import os as _os with patch.dict("os.environ", env, clear=True): - main() + mock_collect.side_effect = make_exc(_os.environ) + with pytest.raises(SystemExit): + main() captured = capsys.readouterr() - assert "super-secret-token-xyz" not in captured.out - assert "super-secret-token-xyz" not in captured.err + assert env["GITEA_TOKEN"] not in captured.out + assert env["GITEA_TOKEN"] not in captured.err diff --git a/tests/test_client.py b/tests/test_client.py index 9c5e827..120c5a1 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -19,6 +19,16 @@ class TestGiteaClientInit: client = GiteaClient("http://gitea.local:3000/", "tok") assert client.base_url == "http://gitea.local:3000" + def test_default_timeout_is_30(self): + """Default timeout is 30 seconds.""" + client = GiteaClient("http://gitea.local:3000", "tok") + assert client.timeout == 30 + + def test_custom_timeout_is_stored(self): + """Custom timeout is stored and used.""" + client = GiteaClient("http://gitea.local:3000", "tok", timeout=10) + assert client.timeout == 10 + class TestGetPaginated: """Test internal pagination logic.""" @@ -117,6 +127,20 @@ class TestGetLatestRelease: assert result is None + def test_raises_on_server_error(self): + """HTTP 500 raises an exception instead of silently returning bad data.""" + import pytest + import requests as req + + client = GiteaClient("http://gitea.local:3000", "tok") + mock_resp = MagicMock() + mock_resp.status_code = 500 + mock_resp.raise_for_status.side_effect = req.HTTPError("500 Server Error") + + with patch.object(client.session, "get", return_value=mock_resp): + with pytest.raises(req.HTTPError): + client.get_latest_release("admin", "my-repo") + class TestGetMilestones: """Test get_milestones method."""