diff --git a/actions/release_homebrew/README.md b/actions/release_homebrew/README.md index 16809f2..0f06d77 100644 --- a/actions/release_homebrew/README.md +++ b/actions/release_homebrew/README.md @@ -35,6 +35,7 @@ steps: | Name | Description | Default | Required | |-------------------------------|-------------------------------------------------------------------------------------------|--------------------------------|----------| | actionlint_config | Contents of actionlint config file, used to override the default brew config. | | `false` | +| additional_trusted_taps | Additional Homebrew taps to trust before validation. | | `false` | | contribute_to_homebrew_core | Whether to contribute to homebrew-core. | `false` | `false` | | formula_file | The full path to the formula file. | | `true` | | git_email | The email to use for the commit. | | `true` | @@ -55,6 +56,8 @@ steps: > [!NOTE] > `org_homebrew_repo` repo name should conform to the documentation. > See: https://docs.brew.sh/Taps#repository-naming-conventions-and-assumptions +> The working tap derived from `org_homebrew_repo` is always trusted automatically. Use `additional_trusted_taps` only +> for extra taps, as a comma or newline-separated list. ## 📤 Outputs diff --git a/actions/release_homebrew/action.yml b/actions/release_homebrew/action.yml index 9f336a9..e129859 100644 --- a/actions/release_homebrew/action.yml +++ b/actions/release_homebrew/action.yml @@ -12,6 +12,12 @@ inputs: description: 'Contents of actionlint config file, used to override the default brew config.' default: '' required: false + additional_trusted_taps: + description: >- + Comma or newline-separated list of additional Homebrew taps to trust before validation. + The working tap from org_homebrew_repo is always trusted automatically. + default: '' + required: false contribute_to_homebrew_core: description: 'Whether to contribute to homebrew-core.' default: 'false' @@ -166,6 +172,7 @@ runs: - name: Homebrew tests env: INPUT_ACTIONLINT_CONFIG: ${{ inputs.actionlint_config }} + INPUT_ADDITIONAL_TRUSTED_TAPS: ${{ inputs.additional_trusted_taps }} INPUT_FORMULA_FILE: ${{ inputs.formula_file }} INPUT_CONTRIBUTE_TO_HOMEBREW_CORE: ${{ inputs.contribute_to_homebrew_core }} INPUT_GIT_EMAIL: ${{ inputs.git_email }} diff --git a/actions/release_homebrew/main.py b/actions/release_homebrew/main.py index 7f8894c..4bb9d02 100644 --- a/actions/release_homebrew/main.py +++ b/actions/release_homebrew/main.py @@ -126,7 +126,7 @@ def _run_subprocess( if exit_code == 0: return True - print(f'::error:: Process [{args_list}] failed with exit code', exit_code) + print('::error:: Process failed with exit code', exit_code) if not ignore_error: ERROR = True return False @@ -762,27 +762,86 @@ def get_test_bot_env(extra_env: Optional[Mapping] = None) -> dict: return env +def parse_additional_trusted_taps(value: str) -> list[str]: + """ + Parse the additional trusted taps input. + + Parameters + ---------- + value : str + Comma or newline-separated tap names. + + Returns + ------- + list[str] + Parsed tap names. + """ + taps = [] + for tap in value.replace(',', '\n').splitlines(): + tap = tap.strip() + if tap: + taps.append(tap) + + return taps + + +def get_trusted_taps() -> list[str]: + """ + Get the ordered list of Homebrew taps to trust. + + Returns + ------- + list[str] + The current tap followed by any additional taps from the action input. + """ + trusted_taps = [ + tap_repo_name, + *parse_additional_trusted_taps(os.getenv('INPUT_ADDITIONAL_TRUSTED_TAPS', '')), + ] + unique_taps = [] + seen_taps = set() + + for trusted_tap in trusted_taps: + trusted_tap = trusted_tap.strip() + trusted_tap_key = trusted_tap.lower() + if not trusted_tap or trusted_tap_key in seen_taps: + continue + + seen_taps.add(trusted_tap_key) + unique_taps.append(trusted_tap) + + return unique_taps + + def trust_tap() -> bool: """ - Trust the configured Homebrew tap. + Trust the configured Homebrew taps. Returns ------- bool - True when Homebrew trusts the tap successfully; otherwise False. + True when Homebrew trusts all taps successfully; otherwise False. """ - start_group(f'Trusting tap {tap_repo_name}') - result = _run_subprocess( - args_list=[ - 'brew', - 'trust', - '--tap', - tap_repo_name, - ], - env=get_test_bot_env(), - ) + trusted_taps = get_trusted_taps() + env = get_test_bot_env() + + start_group('Trusting Homebrew taps') + for index, trusted_tap in enumerate(trusted_taps, start=1): + print(f'Trusting configured Homebrew tap {index} of {len(trusted_taps)}') + if not _run_subprocess( + args_list=[ + 'brew', + 'trust', + '--tap', + trusted_tap, + ], + env=env, + ): + end_group() + return False + end_group() - return result + return True def audit_formula(formula: str) -> bool: @@ -1099,7 +1158,7 @@ def main(): # Trust after cleanup-before, which can reset Homebrew state before doctor runs. if not trust_tap(): - print(f'::error:: Failed to trust tap {tap_repo_name}') + print('::error:: Failed to trust Homebrew taps') raise SystemExit(1) if not brew_test_bot_only_setup(): @@ -1117,6 +1176,12 @@ def main(): print('::error:: brew test-bot --only-tap-syntax failed') FAILURES.append('tap-syntax') + # Refresh trust immediately before formulae validation. test-bot may reset + # or isolate Homebrew state between setup/tap-syntax and dependent checks. + if not trust_tap(): + print('::error:: Failed to trust Homebrew taps before formulae validation') + raise SystemExit(1) + if not brew_test_bot_only_formulae(formula, test_artifacts_dir): print('::error:: brew test-bot --only-formulae failed') FAILURES.append('formulae') diff --git a/tests/release_homebrew/test_release_homebrew.py b/tests/release_homebrew/test_release_homebrew.py index bffd083..d7e0004 100644 --- a/tests/release_homebrew/test_release_homebrew.py +++ b/tests/release_homebrew/test_release_homebrew.py @@ -50,13 +50,19 @@ def test_run_subprocess(capsys, operating_system): def test_run_subprocess_fail(capsys, operating_system): + sensitive_arg = 'secret-value' + result = main._run_subprocess( - args_list=[sys.executable, '-c', 'raise SystemExit(1)'], + args_list=[sys.executable, '-c', 'raise SystemExit(1)', sensitive_arg], ) assert not result, "Process returned zero exit code" assert main.ERROR + captured = capsys.readouterr() + assert sensitive_arg not in captured.out + assert sensitive_arg not in captured.err + @pytest.mark.parametrize('outputs', [ ('test_1', 'foo'), @@ -548,6 +554,34 @@ def test_get_test_bot_env(monkeypatch): assert 'HOMEBREW_USER_CONFIG_HOME' not in env +def test_parse_additional_trusted_taps(): + """Test that additional trusted taps can be comma or newline-separated.""" + trusted_taps = main.parse_additional_trusted_taps( + 'lizardbyte/homebrew-extra, homebrew/core\n\nexample/tap' + ) + + assert trusted_taps == [ + 'lizardbyte/homebrew-extra', + 'homebrew/core', + 'example/tap', + ] + + +def test_get_trusted_taps_includes_current_tap_and_deduplicates(monkeypatch): + """Test that the working tap is always trusted before additional taps.""" + main.tap_repo_name = 'lizardbyte/homebrew' + monkeypatch.setenv( + 'INPUT_ADDITIONAL_TRUSTED_TAPS', + 'homebrew/core,lizardbyte/homebrew\nExample/Tap\nexample/tap', + ) + + assert main.get_trusted_taps() == [ + 'lizardbyte/homebrew', + 'homebrew/core', + 'Example/Tap', + ] + + @patch('actions.release_homebrew.main._run_subprocess') @patch('actions.release_homebrew.main.get_test_bot_env') def test_trust_tap(mock_get_test_bot_env, mock_run_subprocess, capsys): @@ -582,7 +616,111 @@ def test_trust_tap(mock_get_test_bot_env, mock_run_subprocess, capsys): ) captured = capsys.readouterr() - assert 'Trusting tap lizardbyte/homebrew' in captured.out + assert 'Trusting configured Homebrew tap 1 of 1' in captured.out + assert 'lizardbyte/homebrew' not in captured.out + + +@patch('actions.release_homebrew.main._run_subprocess') +@patch('actions.release_homebrew.main.get_test_bot_env') +def test_trust_tap_includes_additional_taps(mock_get_test_bot_env, mock_run_subprocess, monkeypatch): + """ + Test that trust_tap trusts the current tap and additional trusted taps. + + Parameters + ---------- + mock_get_test_bot_env : unittest.mock.Mock + Mocked test-bot environment builder. + mock_run_subprocess : unittest.mock.Mock + Mocked subprocess runner. + monkeypatch : pytest.MonkeyPatch + Fixture used to patch environment variables. + """ + main.tap_repo_name = 'lizardbyte/homebrew' + monkeypatch.setenv('INPUT_ADDITIONAL_TRUSTED_TAPS', 'homebrew/core\nexample/tap') + mock_get_test_bot_env.return_value = { + 'HOME': os.path.join(os.getcwd(), 'runner-home'), + } + mock_run_subprocess.return_value = True + + assert main.trust_tap() + + assert [call.kwargs['args_list'] for call in mock_run_subprocess.call_args_list] == [ + [ + 'brew', + 'trust', + '--tap', + 'lizardbyte/homebrew', + ], + [ + 'brew', + 'trust', + '--tap', + 'homebrew/core', + ], + [ + 'brew', + 'trust', + '--tap', + 'example/tap', + ], + ] + assert all( + call.kwargs['env'] == mock_get_test_bot_env.return_value + for call in mock_run_subprocess.call_args_list + ) + + +@patch('actions.release_homebrew.main._run_subprocess') +@patch('actions.release_homebrew.main.get_test_bot_env') +def test_trust_tap_stops_when_a_tap_fails_without_logging_tap_names( + mock_get_test_bot_env, + mock_run_subprocess, + monkeypatch, + capsys, +): + """ + Test that trust_tap stops on failure without logging tap names. + + Parameters + ---------- + mock_get_test_bot_env : unittest.mock.Mock + Mocked test-bot environment builder. + mock_run_subprocess : unittest.mock.Mock + Mocked subprocess runner. + monkeypatch : pytest.MonkeyPatch + Fixture used to patch environment variables. + capsys : pytest.CaptureFixture + Fixture used to inspect the log output. + """ + main.tap_repo_name = 'lizardbyte/homebrew' + monkeypatch.setenv('INPUT_ADDITIONAL_TRUSTED_TAPS', 'homebrew/core') + mock_get_test_bot_env.return_value = { + 'HOME': os.path.join(os.getcwd(), 'runner-home'), + } + mock_run_subprocess.side_effect = [True, False] + + assert not main.trust_tap() + + assert [call.kwargs['args_list'] for call in mock_run_subprocess.call_args_list] == [ + [ + 'brew', + 'trust', + '--tap', + 'lizardbyte/homebrew', + ], + [ + 'brew', + 'trust', + '--tap', + 'homebrew/core', + ], + ] + + captured = capsys.readouterr() + assert 'Trusting configured Homebrew tap 1 of 2' in captured.out + assert 'Trusting configured Homebrew tap 2 of 2' in captured.out + assert 'lizardbyte/homebrew' not in captured.out + assert 'homebrew/core' not in captured.out @patch('actions.release_homebrew.main._run_subprocess') @@ -954,9 +1092,9 @@ def test_main(brew_untap, org_homebrew_repo, homebrew_core_fork_repo, input_vali assert not main.FAILURES -def test_main_trusts_tap_after_cleanup(monkeypatch): +def test_main_trusts_tap_after_cleanup_and_before_formulae(monkeypatch): """ - Test that main trusts the tap after Homebrew cleanup and before setup. + Test that main trusts the tap after Homebrew cleanup and before formulae validation. Parameters ---------- @@ -998,6 +1136,63 @@ def _record(*args, **kwargs): assert calls.index('brew_test_bot_only_cleanup_before') < calls.index('trust_tap') assert calls.index('setup_linux_sandbox') < calls.index('trust_tap') assert calls.index('trust_tap') < calls.index('brew_test_bot_only_setup') + assert calls.count('trust_tap') == 2 + + first_trust_index = calls.index('trust_tap') + second_trust_index = len(calls) - 1 - calls[::-1].index('trust_tap') + + assert first_trust_index < calls.index('brew_test_bot_only_setup') + assert calls.index('brew_test_bot_only_tap_syntax') < second_trust_index + assert second_trust_index < calls.index('brew_test_bot_only_formulae') + + +def test_main_fails_when_formulae_trust_refresh_fails(monkeypatch): + """ + Test that main stops before formulae validation when the second trust refresh fails. + + Parameters + ---------- + monkeypatch : pytest.MonkeyPatch + Fixture used to patch environment variables and action steps. + """ + monkeypatch.setenv('INPUT_VALIDATE', 'true') + main.ERROR = False + main.FAILURES = [] + main.args = main._parse_args([]) + calls = [] + + def record_call(name, return_value=True): + def _record(*args, **kwargs): + calls.append(name) + return return_value + + return _record + + def trust_once_then_fail(*args, **kwargs): + calls.append('trust_tap') + return calls.count('trust_tap') == 1 + + monkeypatch.setattr(main, 'is_brew_installed', record_call('is_brew_installed')) + monkeypatch.setattr(main, 'process_input_formula', record_call('process_input_formula', 'hello_world')) + monkeypatch.setattr( + main, + 'brew_test_bot_only_cleanup_before', + record_call('brew_test_bot_only_cleanup_before'), + ) + monkeypatch.setattr(main, 'setup_linux_sandbox', record_call('setup_linux_sandbox')) + monkeypatch.setattr(main, 'trust_tap', trust_once_then_fail) + monkeypatch.setattr(main, 'brew_test_bot_only_setup', record_call('brew_test_bot_only_setup')) + monkeypatch.setattr(main, 'audit_formula', record_call('audit_formula')) + monkeypatch.setattr(main, 'override_actionlint_config', record_call('override_actionlint_config')) + monkeypatch.setattr(main, 'brew_test_bot_only_tap_syntax', record_call('brew_test_bot_only_tap_syntax')) + monkeypatch.setattr(main, 'prepare_test_artifacts_dir', record_call('prepare_test_artifacts_dir', 'test-path')) + monkeypatch.setattr(main, 'brew_test_bot_only_formulae', record_call('brew_test_bot_only_formulae')) + + with pytest.raises(SystemExit): + main.main() + + assert calls.count('trust_tap') == 2 + assert 'brew_test_bot_only_formulae' not in calls @pytest.mark.parametrize('scenario, mocks, expected_failures', [