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 <noreply@anthropic.com>
This commit is contained in:
@@ -41,18 +41,18 @@ def main() -> None:
|
|||||||
repos = collect_all(client)
|
repos = collect_all(client)
|
||||||
except requests.ConnectionError:
|
except requests.ConnectionError:
|
||||||
console.print("[red]Erreur : connexion refusee. Verifiez l'URL et le serveur Gitea.[/red]")
|
console.print("[red]Erreur : connexion refusee. Verifiez l'URL et le serveur Gitea.[/red]")
|
||||||
return
|
sys.exit(1)
|
||||||
except requests.Timeout:
|
except requests.Timeout:
|
||||||
console.print(
|
console.print(
|
||||||
"[red]Erreur : delai d'attente depasse. Le serveur Gitea ne repond pas.[/red]"
|
"[red]Erreur : delai d'attente depasse. Le serveur Gitea ne repond pas.[/red]"
|
||||||
)
|
)
|
||||||
return
|
sys.exit(1)
|
||||||
except requests.RequestException as exc:
|
except requests.RequestException as exc:
|
||||||
# Ne jamais afficher le token dans les messages d'erreur
|
# Ne jamais afficher le token dans les messages d'erreur
|
||||||
msg = str(exc)
|
msg = str(exc)
|
||||||
if token in msg:
|
if token in msg:
|
||||||
msg = msg.replace(token, "***")
|
msg = msg.replace(token, "***")
|
||||||
console.print(f"[red]Erreur API : {msg}[/red]")
|
console.print(f"[red]Erreur API : {msg}[/red]")
|
||||||
return
|
sys.exit(1)
|
||||||
|
|
||||||
render_dashboard(repos)
|
render_dashboard(repos)
|
||||||
|
|||||||
@@ -14,9 +14,16 @@ class GiteaClient:
|
|||||||
|
|
||||||
_PAGE_LIMIT = 50
|
_PAGE_LIMIT = 50
|
||||||
|
|
||||||
def __init__(self, base_url: str, token: str) -> None:
|
def __init__(self, base_url: str, token: str, timeout: int = 30) -> None:
|
||||||
"""Initialise le client avec l'URL de base et le token API."""
|
"""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.base_url = base_url.rstrip("/")
|
||||||
|
self.timeout = timeout
|
||||||
self.session = requests.Session()
|
self.session = requests.Session()
|
||||||
self.session.headers["Authorization"] = f"token {token}"
|
self.session.headers["Authorization"] = f"token {token}"
|
||||||
|
|
||||||
@@ -33,7 +40,7 @@ class GiteaClient:
|
|||||||
merged_params["limit"] = self._PAGE_LIMIT
|
merged_params["limit"] = self._PAGE_LIMIT
|
||||||
merged_params["page"] = page
|
merged_params["page"] = page
|
||||||
url = f"{self.base_url}{endpoint}"
|
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()
|
resp.raise_for_status()
|
||||||
items = resp.json()
|
items = resp.json()
|
||||||
all_items.extend(items)
|
all_items.extend(items)
|
||||||
@@ -57,9 +64,10 @@ class GiteaClient:
|
|||||||
Gere HTTP 404 en retournant None.
|
Gere HTTP 404 en retournant None.
|
||||||
"""
|
"""
|
||||||
url = f"{self.base_url}/api/v1/repos/{owner}/{repo}/releases/latest"
|
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:
|
if resp.status_code == 404:
|
||||||
return None
|
return None
|
||||||
|
resp.raise_for_status()
|
||||||
return resp.json()
|
return resp.json()
|
||||||
|
|
||||||
def get_milestones(self, owner: str, repo: str) -> list[dict]:
|
def get_milestones(self, owner: str, repo: str) -> list[dict]:
|
||||||
|
|||||||
@@ -46,17 +46,15 @@ class TestMainNominal:
|
|||||||
class TestMainMissingToken:
|
class TestMainMissingToken:
|
||||||
"""Test main() when GITEA_TOKEN is not set."""
|
"""Test main() when GITEA_TOKEN is not set."""
|
||||||
|
|
||||||
def test_exits_when_token_missing(self):
|
def test_error_message_when_token_missing(self, capsys):
|
||||||
"""main() exits with SystemExit when GITEA_TOKEN is absent."""
|
"""main() exits with code 1 and prints message mentioning GITEA_TOKEN."""
|
||||||
with patch.dict("os.environ", {}, clear=True):
|
with patch.dict("os.environ", {}, clear=True):
|
||||||
with pytest.raises(SystemExit):
|
with pytest.raises(SystemExit) as exc_info:
|
||||||
main()
|
main()
|
||||||
|
|
||||||
def test_error_message_when_token_missing(self, capsys):
|
assert exc_info.value.code == 1
|
||||||
"""main() prints a clear error message about missing token."""
|
captured = capsys.readouterr()
|
||||||
with patch.dict("os.environ", {}, clear=True):
|
assert "GITEA_TOKEN" in captured.err
|
||||||
with pytest.raises(SystemExit):
|
|
||||||
main()
|
|
||||||
|
|
||||||
|
|
||||||
class TestMainConnectionErrors:
|
class TestMainConnectionErrors:
|
||||||
@@ -65,48 +63,65 @@ class TestMainConnectionErrors:
|
|||||||
@patch("gitea_dashboard.cli.collect_all")
|
@patch("gitea_dashboard.cli.collect_all")
|
||||||
@patch("gitea_dashboard.cli.GiteaClient")
|
@patch("gitea_dashboard.cli.GiteaClient")
|
||||||
def test_connection_error_handled(self, mock_client_cls, mock_collect):
|
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"}
|
env = {"GITEA_TOKEN": "test-token"}
|
||||||
mock_client_cls.return_value = MagicMock()
|
mock_client_cls.return_value = MagicMock()
|
||||||
mock_collect.side_effect = requests.ConnectionError("Connection refused")
|
mock_collect.side_effect = requests.ConnectionError("Connection refused")
|
||||||
|
|
||||||
with patch.dict("os.environ", env, clear=True):
|
with patch.dict("os.environ", env, clear=True):
|
||||||
# Should not raise — error is handled gracefully
|
with pytest.raises(SystemExit) as exc_info:
|
||||||
main()
|
main()
|
||||||
|
|
||||||
|
assert exc_info.value.code == 1
|
||||||
|
|
||||||
@patch("gitea_dashboard.cli.collect_all")
|
@patch("gitea_dashboard.cli.collect_all")
|
||||||
@patch("gitea_dashboard.cli.GiteaClient")
|
@patch("gitea_dashboard.cli.GiteaClient")
|
||||||
def test_timeout_error_handled(self, mock_client_cls, mock_collect):
|
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"}
|
env = {"GITEA_TOKEN": "test-token"}
|
||||||
mock_client_cls.return_value = MagicMock()
|
mock_client_cls.return_value = MagicMock()
|
||||||
mock_collect.side_effect = requests.Timeout("Request timed out")
|
mock_collect.side_effect = requests.Timeout("Request timed out")
|
||||||
|
|
||||||
with patch.dict("os.environ", env, clear=True):
|
with patch.dict("os.environ", env, clear=True):
|
||||||
|
with pytest.raises(SystemExit) as exc_info:
|
||||||
main()
|
main()
|
||||||
|
|
||||||
|
assert exc_info.value.code == 1
|
||||||
|
|
||||||
@patch("gitea_dashboard.cli.collect_all")
|
@patch("gitea_dashboard.cli.collect_all")
|
||||||
@patch("gitea_dashboard.cli.GiteaClient")
|
@patch("gitea_dashboard.cli.GiteaClient")
|
||||||
def test_request_exception_handled(self, mock_client_cls, mock_collect):
|
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"}
|
env = {"GITEA_TOKEN": "test-token"}
|
||||||
mock_client_cls.return_value = MagicMock()
|
mock_client_cls.return_value = MagicMock()
|
||||||
mock_collect.side_effect = requests.RequestException("Something went wrong")
|
mock_collect.side_effect = requests.RequestException("Something went wrong")
|
||||||
|
|
||||||
with patch.dict("os.environ", env, clear=True):
|
with patch.dict("os.environ", env, clear=True):
|
||||||
|
with pytest.raises(SystemExit) as exc_info:
|
||||||
main()
|
main()
|
||||||
|
|
||||||
|
assert exc_info.value.code == 1
|
||||||
|
|
||||||
@patch("gitea_dashboard.cli.collect_all")
|
@patch("gitea_dashboard.cli.collect_all")
|
||||||
@patch("gitea_dashboard.cli.GiteaClient")
|
@patch("gitea_dashboard.cli.GiteaClient")
|
||||||
def test_token_not_in_error_output(self, mock_client_cls, mock_collect, capsys):
|
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"}
|
env = {"GITEA_TOKEN": "super-secret-token-xyz"}
|
||||||
mock_client_cls.return_value = MagicMock()
|
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):
|
with patch.dict("os.environ", env, clear=True):
|
||||||
|
mock_collect.side_effect = make_exc(_os.environ)
|
||||||
|
with pytest.raises(SystemExit):
|
||||||
main()
|
main()
|
||||||
|
|
||||||
captured = capsys.readouterr()
|
captured = capsys.readouterr()
|
||||||
assert "super-secret-token-xyz" not in captured.out
|
assert env["GITEA_TOKEN"] not in captured.out
|
||||||
assert "super-secret-token-xyz" not in captured.err
|
assert env["GITEA_TOKEN"] not in captured.err
|
||||||
|
|||||||
@@ -19,6 +19,16 @@ class TestGiteaClientInit:
|
|||||||
client = GiteaClient("http://gitea.local:3000/", "tok")
|
client = GiteaClient("http://gitea.local:3000/", "tok")
|
||||||
assert client.base_url == "http://gitea.local:3000"
|
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:
|
class TestGetPaginated:
|
||||||
"""Test internal pagination logic."""
|
"""Test internal pagination logic."""
|
||||||
@@ -117,6 +127,20 @@ class TestGetLatestRelease:
|
|||||||
|
|
||||||
assert result is None
|
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:
|
class TestGetMilestones:
|
||||||
"""Test get_milestones method."""
|
"""Test get_milestones method."""
|
||||||
|
|||||||
Reference in New Issue
Block a user