diff --git a/cli/localci/cli/run.py b/cli/localci/cli/run.py index f58ab30..470a0b0 100644 --- a/cli/localci/cli/run.py +++ b/cli/localci/cli/run.py @@ -7,11 +7,11 @@ from __future__ import annotations import os +import re import tempfile from pathlib import Path import click -import re from localci.core.executor import ( ActNotFoundError, @@ -29,6 +29,7 @@ from localci.core.queue_builder import QueueBuilder from localci.core.results import ExecutionSummary from localci.core.workflow import MatrixEntry, Platform, WorkflowAnalyzer +from localci.core.github_token import resolve_github_token, warn_sentinel_github_token from localci.core.boost_cache import ensure_boost_cache from localci.core.ccache_stats import get_ccache_stats from localci.core.config import resolve_cache_paths @@ -156,7 +157,9 @@ def run( workflow_path = Path(workflow) if workflow else cfg.workflow project_dir = Path(".").resolve() - gh_token = github_token or os.environ.get("GITHUB_TOKEN") or "local-ci-token" + gh_token = resolve_github_token(github_token) + if not offline: + warn_sentinel_github_token(gh_token) # ── 1. Parse the workflow ────────────────────────────────────── try: diff --git a/cli/localci/core/command_builder.py b/cli/localci/core/command_builder.py index 1b2c207..085743a 100644 --- a/cli/localci/core/command_builder.py +++ b/cli/localci/core/command_builder.py @@ -16,6 +16,7 @@ from localci.core.config import CacheConfig, ResolvedCachePaths from localci.core.executor import ActCommand +from localci.core.github_token import SENTINEL_GITHUB_TOKEN from localci.core.workflow import MatrixEntry logger = logging.getLogger(__name__) @@ -175,9 +176,10 @@ def build( if resolved_cache_paths.b2_source_host is not None: env["LOCALCI_B2_SOURCE_DIR"] = resolved_cache_paths.b2_source_container - # Secrets + # Secrets: copy caller-provided secrets; only fill GITHUB_TOKEN when absent + # (setdefault — never override a real token from the orchestrator path). secrets = {**self.default_secrets} - secrets.setdefault("GITHUB_TOKEN", "local-ci-token") + secrets.setdefault("GITHUB_TOKEN", SENTINEL_GITHUB_TOKEN) # Architecture: request linux/386 only when using a generic image # (e.g. ubuntu:24.04). Our capy x86 image is amd64 with multilib, so diff --git a/cli/localci/core/executor.py b/cli/localci/core/executor.py index 20d246c..1cbed47 100644 --- a/cli/localci/core/executor.py +++ b/cli/localci/core/executor.py @@ -9,6 +9,7 @@ import logging import os +import re import shutil import subprocess import sys @@ -23,6 +24,28 @@ logger = logging.getLogger(__name__) +# Substrings (matched case-insensitively) for summarizing failed job output. +_ERROR_EXTRACT_KEYWORDS = ( + "error:", + "fatal:", + "failed", + "error[", + "undefined reference", + "no such file", + "cannot find", + "compilation failed", +) + +# Public for tests: substring signals for auth/API failures in act output. +# HTTP 4xx status codes use _HTTP_STATUS_PATTERN (word-boundary) to avoid +# false positives such as "4010" or "port 40100". +AUTH_ERROR_EXTRACT_KEYWORDS = ( + "unauthorized", + "forbidden", + "rate limit", +) +_HTTP_STATUS_PATTERN = re.compile(r"\b4\d{2}\b") + # ===================================================================== # Enums @@ -621,6 +644,16 @@ def _get_log_path(self, matrix_name: str) -> Path: filename = f"{timestamp}_{safe_name}.log" return self.logs_dir / filename + @staticmethod + def _line_indicates_error(line: str) -> bool: + """Return True if *line* looks like a failed-job error summary.""" + lower = line.lower() + if any(kw in lower for kw in _ERROR_EXTRACT_KEYWORDS): + return True + if any(kw in lower for kw in AUTH_ERROR_EXTRACT_KEYWORDS): + return True + return _HTTP_STATUS_PATTERN.search(line) is not None + @staticmethod def _extract_error(output: str, max_lines: int = 10) -> str: """Extract error summary from output.""" @@ -628,20 +661,7 @@ def _extract_error(output: str, max_lines: int = 10) -> str: error_lines: list[str] = [] for line in lines: - lower = line.lower() - if any( - kw in lower - for kw in ( - "error:", - "fatal:", - "failed", - "error[", - "undefined reference", - "no such file", - "cannot find", - "compilation failed", - ) - ): + if JobExecutor._line_indicates_error(line): error_lines.append(line.strip()) if error_lines: diff --git a/cli/localci/core/github_token.py b/cli/localci/core/github_token.py new file mode 100644 index 0000000..8fd7472 --- /dev/null +++ b/cli/localci/core/github_token.py @@ -0,0 +1,48 @@ +"""GitHub token resolution for act and workflow action downloads. + +When no GitHub token is provided, act falls back to a non-functional +placeholder, which produces opaque HTTP 401 errors during action downloads. +This module surfaces that condition as an early, visible warning. +""" + +from __future__ import annotations + +import os + +from localci.utils.output import print_important_warning + +SENTINEL_GITHUB_TOKEN = "local-ci-token" + + +def resolve_github_token(cli_token: str | None) -> str: + """Return CLI token, ``GITHUB_TOKEN`` env, or the local-ci sentinel.""" + if cli_token is not None: + stripped = cli_token.strip() + if stripped: + return stripped + env_token = os.environ.get("GITHUB_TOKEN") + if env_token and env_token.strip(): + return env_token.strip() + return SENTINEL_GITHUB_TOKEN + + +def is_sentinel_github_token(token: str) -> bool: + """True when *token* is the non-functional placeholder used for local runs.""" + return token == SENTINEL_GITHUB_TOKEN + + +def format_sentinel_github_token_warning() -> str: + """User-facing warning when falling back to :data:`SENTINEL_GITHUB_TOKEN`.""" + return ( + "No GitHub token provided; using placeholder token for act. " + "Action downloads may fail with HTTP 401. Set a real token via: " + "export GITHUB_TOKEN=ghp_... , " + "localci run --github-token ghp_... , " + "or use --offline if actions are already cached." + ) + + +def warn_sentinel_github_token(token: str) -> None: + """Emit a Rich console warning when *token* is :data:`SENTINEL_GITHUB_TOKEN`.""" + if is_sentinel_github_token(token): + print_important_warning(format_sentinel_github_token_warning()) diff --git a/cli/localci/utils/output.py b/cli/localci/utils/output.py index f358d44..8d3cf04 100644 --- a/cli/localci/utils/output.py +++ b/cli/localci/utils/output.py @@ -6,6 +6,8 @@ from __future__ import annotations +import sys + from rich.console import Console from rich.panel import Panel from rich.table import Table @@ -28,18 +30,29 @@ } ) -# Module-level console (reconfigured by ``configure_console``). -console = Console(theme=LOCALCI_THEME) +# Module-level consoles (reconfigured by ``configure_console``). +console = Console(theme=LOCALCI_THEME, no_color=False) +# High-severity messages (e.g. missing GitHub token) bypass ``--quiet``. +# Bind to sys.stdout so each print uses the current stream (pytest, CliRunner). +_important_console = Console( + theme=LOCALCI_THEME, file=sys.stdout, no_color=False, quiet=False +) def configure_console(*, no_color: bool = False, quiet: bool = False) -> None: """Reconfigure the global *console* based on CLI flags.""" - global console + global console, _important_console console = Console( theme=LOCALCI_THEME, no_color=no_color, quiet=quiet, ) + _important_console = Console( + theme=LOCALCI_THEME, + file=sys.stdout, + no_color=no_color, + quiet=False, + ) # --------------------------------------------------------------------------- @@ -64,6 +77,11 @@ def print_warning(message: str) -> None: console.print(f"[warning]![/warning] {message}") +def print_important_warning(message: str) -> None: + """Print a warning that is still shown when ``--quiet`` is set.""" + _important_console.print(f"[warning]![/warning] {message}") + + def print_info(message: str) -> None: console.print(f"[info]ℹ[/info] {message}") diff --git a/cli/tests/test_executor.py b/cli/tests/test_executor.py index fea1c6f..9f47681 100644 --- a/cli/tests/test_executor.py +++ b/cli/tests/test_executor.py @@ -455,16 +455,21 @@ def test_default_status_is_pending(self): class TestJobExecutor: """Test executor with mocked subprocess.""" + @pytest.fixture(autouse=True) + def _logs_dir(self, tmp_path: Path) -> None: + self.logs_dir = tmp_path / "logs" + self.logs_dir.mkdir(parents=True, exist_ok=True) + @patch("shutil.which") def test_has_act_true(self, mock_which): mock_which.return_value = "/usr/bin/act" - executor = JobExecutor(logs_dir=Path("/tmp/localci-test")) + executor = JobExecutor(logs_dir=self.logs_dir) assert executor.has_act is True @patch("shutil.which") def test_has_act_false(self, mock_which): mock_which.return_value = None - executor = JobExecutor(logs_dir=Path("/tmp/localci-test")) + executor = JobExecutor(logs_dir=self.logs_dir) assert executor.has_act is False @patch("sys.platform", "win32") @@ -479,7 +484,7 @@ def which_side_effect(name): return None mock_which.side_effect = which_side_effect - executor = JobExecutor(logs_dir=Path("/tmp/localci-test")) + executor = JobExecutor(logs_dir=self.logs_dir) assert executor.has_act is True assert executor._act_path == "C:\\ProgramData\\chocolatey\\bin\\act-cli.exe" @@ -490,14 +495,14 @@ def test_check_act_success(self, mock_which, mock_run): mock_run.return_value = MagicMock( returncode=0, stdout="act version 0.2.68" ) - executor = JobExecutor(logs_dir=Path("/tmp/localci-test")) + executor = JobExecutor(logs_dir=self.logs_dir) version = executor.check_act() assert "0.2.68" in version @patch("shutil.which") def test_check_act_not_installed(self, mock_which): mock_which.return_value = None - executor = JobExecutor(logs_dir=Path("/tmp/localci-test")) + executor = JobExecutor(logs_dir=self.logs_dir) with pytest.raises(ActNotFoundError, match="act is not installed"): executor.check_act() @@ -506,7 +511,7 @@ def test_check_act_not_installed(self, mock_which): def test_check_docker_success(self, mock_which, mock_run): mock_which.side_effect = lambda name: f"/usr/bin/{name}" mock_run.return_value = MagicMock(returncode=0) - executor = JobExecutor(logs_dir=Path("/tmp/localci-test")) + executor = JobExecutor(logs_dir=self.logs_dir) # Should not raise executor.check_docker() @@ -515,7 +520,7 @@ def test_check_docker_not_installed(self, mock_which): mock_which.side_effect = ( lambda name: "/usr/bin/act" if name == "act" else None ) - executor = JobExecutor(logs_dir=Path("/tmp/localci-test")) + executor = JobExecutor(logs_dir=self.logs_dir) with pytest.raises( DockerNotAvailableError, match="not installed" ): @@ -526,14 +531,14 @@ def test_check_docker_not_installed(self, mock_which): def test_check_docker_not_running(self, mock_which, mock_run): mock_which.side_effect = lambda name: f"/usr/bin/{name}" mock_run.return_value = MagicMock(returncode=1) - executor = JobExecutor(logs_dir=Path("/tmp/localci-test")) + executor = JobExecutor(logs_dir=self.logs_dir) with pytest.raises(DockerNotAvailableError, match="not running"): executor.check_docker() @patch("shutil.which") def test_run_dryrun(self, mock_which): mock_which.side_effect = lambda name: f"/usr/bin/{name}" - executor = JobExecutor(logs_dir=Path("/tmp/localci-test")) + executor = JobExecutor(logs_dir=self.logs_dir) cmd = ActCommand( workflow_file=Path("ci.yml"), @@ -555,7 +560,7 @@ def test_run_dryrun(self, mock_which): @patch("shutil.which") def test_run_preflight_act_fails(self, mock_which): mock_which.return_value = None - executor = JobExecutor(logs_dir=Path("/tmp/localci-test")) + executor = JobExecutor(logs_dir=self.logs_dir) cmd = ActCommand( workflow_file=Path("ci.yml"), @@ -570,7 +575,7 @@ def test_run_preflight_docker_fails(self, mock_which): mock_which.side_effect = ( lambda name: "/usr/bin/act" if name == "act" else None ) - executor = JobExecutor(logs_dir=Path("/tmp/localci-test")) + executor = JobExecutor(logs_dir=self.logs_dir) cmd = ActCommand( workflow_file=Path("ci.yml"), @@ -588,7 +593,7 @@ def test_run_preflight_docker_fails(self, mock_which): @patch("shutil.which") def test_extract_error_finds_keywords(self, mock_which): mock_which.return_value = "/usr/bin/act" - executor = JobExecutor(logs_dir=Path("/tmp/localci-test")) + executor = JobExecutor(logs_dir=self.logs_dir) output = textwrap.dedent("""\ Step 1: setup Step 2: build @@ -603,12 +608,62 @@ def test_extract_error_finds_keywords(self, mock_which): @patch("shutil.which") def test_extract_error_fallback(self, mock_which): mock_which.return_value = "/usr/bin/act" - executor = JobExecutor(logs_dir=Path("/tmp/localci-test")) + executor = JobExecutor(logs_dir=self.logs_dir) output = "line 1\nline 2\nline 3" extracted = executor._extract_error(output, max_lines=2) assert "line 2" in extracted assert "line 3" in extracted + @patch("shutil.which") + def test_extract_error_http_401(self, mock_which): + mock_which.return_value = "/usr/bin/act" + executor = JobExecutor(logs_dir=self.logs_dir) + # Auth line not in last two lines so max_lines=2 fallback cannot pass alone. + output = ( + "setup: resolving action\n" + "HTTP 401: unauthorized - Bad credentials\n" + "middle: post-download\n" + "all done\n" + "finished ok" + ) + extracted = executor._extract_error(output, max_lines=2) + assert "401" in extracted + assert "unauthorized" in extracted.lower() + assert "finished ok" not in extracted + + @patch("shutil.which") + def test_extract_error_http_403(self, mock_which): + mock_which.return_value = "/usr/bin/act" + executor = JobExecutor(logs_dir=self.logs_dir) + output = ( + "setup: resolving action\n" + "received HTTP status: 403 forbidden for this resource\n" + "middle: post-download\n" + "all done\n" + "finished ok" + ) + extracted = executor._extract_error(output, max_lines=2) + assert "403" in extracted + assert "forbidden" in extracted.lower() + assert "finished ok" not in extracted + + @patch("shutil.which") + def test_extract_error_ignores_401_false_positive(self, mock_which): + mock_which.return_value = "/usr/bin/act" + executor = JobExecutor(logs_dir=self.logs_dir) + # 4010 line must not be in the last two lines so max_lines=2 fallback + # differs from wrongly treating "401" inside "4010" as an error line. + output = ( + "progress: fetched 4010 bytes from cache\n" + "middle: still running\n" + "all done\n" + "finished ok" + ) + extracted = executor._extract_error(output, max_lines=2) + assert "4010" not in extracted + assert "all done" in extracted + assert "finished ok" in extracted + def test_cleanup_temp_files(self, tmp_path): event = tmp_path / "event.json" event.write_text("{}") @@ -674,6 +729,8 @@ def test_extra_env(self, tmp_path): assert cmd.env["BOOST_ROOT"] == "/opt/boost" def test_default_secrets(self, tmp_path): + from localci.core.github_token import SENTINEL_GITHUB_TOKEN + wf = tmp_path / "ci.yml" wf.write_text("name: CI") @@ -681,7 +738,7 @@ def test_default_secrets(self, tmp_path): entry = _make_entry() cmd = builder.build(entry) - assert "GITHUB_TOKEN" in cmd.secrets + assert cmd.secrets["GITHUB_TOKEN"] == SENTINEL_GITHUB_TOKEN def test_custom_default_secrets(self, tmp_path): wf = tmp_path / "ci.yml" diff --git a/cli/tests/test_github_token.py b/cli/tests/test_github_token.py new file mode 100644 index 0000000..bd9706e --- /dev/null +++ b/cli/tests/test_github_token.py @@ -0,0 +1,183 @@ +"""Tests for GitHub token resolution and sentinel warnings.""" + +from __future__ import annotations + +import textwrap +from pathlib import Path + +import pytest +from click.testing import CliRunner + +from localci.cli.main import cli +from localci.core.executor import AUTH_ERROR_EXTRACT_KEYWORDS, JobExecutor +from localci.core.github_token import ( + SENTINEL_GITHUB_TOKEN, + format_sentinel_github_token_warning, + is_sentinel_github_token, + resolve_github_token, +) +runner = CliRunner() +SAMPLE_WORKFLOW = str(Path(__file__).parent / "fixtures" / "sample_workflow.yml") + + +class TestResolveGithubToken: + def test_cli_token_takes_precedence(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("GITHUB_TOKEN", "env-token") + assert resolve_github_token("cli-token") == "cli-token" + + def test_env_token_when_no_cli(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("GITHUB_TOKEN", "env-token") + assert resolve_github_token(None) == "env-token" + + def test_sentinel_when_unset(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("GITHUB_TOKEN", raising=False) + assert resolve_github_token(None) == SENTINEL_GITHUB_TOKEN + + def test_blank_cli_token_falls_back_to_sentinel( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.delenv("GITHUB_TOKEN", raising=False) + assert resolve_github_token(" ") == SENTINEL_GITHUB_TOKEN + + def test_blank_env_token_falls_back_to_sentinel( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("GITHUB_TOKEN", " ") + assert resolve_github_token(None) == SENTINEL_GITHUB_TOKEN + + def test_env_token_is_stripped(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("GITHUB_TOKEN", " env-token ") + assert resolve_github_token(None) == "env-token" + + def test_is_sentinel_detects_placeholder(self) -> None: + assert is_sentinel_github_token(SENTINEL_GITHUB_TOKEN) + assert not is_sentinel_github_token("ghp_real") + + def test_warning_message_documents_remediation(self) -> None: + msg = format_sentinel_github_token_warning() + assert "GITHUB_TOKEN" in msg + assert "--github-token" in msg + assert "--offline" in msg + assert "401" in msg + +class TestAuthErrorExtractKeywords: + def test_auth_keywords_registered(self) -> None: + assert AUTH_ERROR_EXTRACT_KEYWORDS == ( + "unauthorized", + "forbidden", + "rate limit", + ) + + +class TestExtractErrorAuthKeywords: + @pytest.mark.parametrize( + "line", + [ + "Error: HTTP 401 Unauthorized", + "authentication required: unauthorized", + "403 Forbidden: resource not accessible", + "received HTTP status: 403", + "API rate limit exceeded for user", + ], + ) + def test_extract_error_matches_auth_keywords(self, line: str) -> None: + output = textwrap.dedent(f"""\ + Downloading action + {line} + cleanup + """) + extracted = JobExecutor._extract_error(output) + assert line.strip() in extracted + + +class TestRunSentinelWarning: + def test_dry_run_warns_when_no_token( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.delenv("GITHUB_TOKEN", raising=False) + result = runner.invoke( + cli, + ["run", "--workflow", SAMPLE_WORKFLOW, "--dry-run", "--platform", "linux"], + ) + assert result.exit_code == 0, result.output + assert "No GitHub token provided" in result.output + assert "GITHUB_TOKEN" in result.output + + def test_dry_run_warns_when_no_token_and_quiet( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.delenv("GITHUB_TOKEN", raising=False) + result = runner.invoke( + cli, + [ + "-q", + "run", + "--workflow", + SAMPLE_WORKFLOW, + "--dry-run", + "--platform", + "linux", + ], + ) + assert result.exit_code == 0, result.output + assert "No GitHub token provided" in result.output + + def test_dry_run_no_warning_when_offline( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.delenv("GITHUB_TOKEN", raising=False) + result = runner.invoke( + cli, + [ + "run", + "--workflow", + SAMPLE_WORKFLOW, + "--dry-run", + "--platform", + "linux", + "--offline", + ], + ) + assert result.exit_code == 0, result.output + assert "No GitHub token provided" not in result.output + + def test_dry_run_no_warning_with_cli_token( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.delenv("GITHUB_TOKEN", raising=False) + result = runner.invoke( + cli, + [ + "run", + "--workflow", + SAMPLE_WORKFLOW, + "--dry-run", + "--platform", + "linux", + "--github-token", + "ghp_test_token", + ], + ) + assert result.exit_code == 0, result.output + assert "No GitHub token provided" not in result.output + + def test_early_exit_still_warns_when_no_jobs_match( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.delenv("GITHUB_TOKEN", raising=False) + result = runner.invoke( + cli, + [ + "run", + "--workflow", + SAMPLE_WORKFLOW, + "--dry-run", + "--platform", + "linux", + "--job", + "nonexistent-job-xyz", + ], + ) + assert result.exit_code == 0, result.output + assert "No GitHub token provided" in result.output + assert "No jobs match" in result.output