diff --git a/lib/ghb/gitignore_manager.rb b/lib/ghb/gitignore_manager.rb index 4820bf7..b006b9b 100644 --- a/lib/ghb/gitignore_manager.rb +++ b/lib/ghb/gitignore_manager.rb @@ -38,7 +38,13 @@ def update api_url = "https://www.toptal.com/developers/gitignore/api/#{templates_param}" puts(" Detected templates: #{detected_templates.join(', ')}") - response = HTTParty.get(api_url, timeout: 30) + + response = + begin + HTTParty.get(api_url, timeout: 30) + rescue Net::OpenTimeout, Net::ReadTimeout, Errno::ECONNRESET, Errno::ECONNREFUSED, SocketError => e + raise("Cannot fetch gitignore templates: #{e.class}: #{e.message}") + end raise("Cannot fetch gitignore templates: #{response.message}") unless response.code == 200 diff --git a/lib/ghb/repository_configurator.rb b/lib/ghb/repository_configurator.rb index ed82cf3..991934f 100644 --- a/lib/ghb/repository_configurator.rb +++ b/lib/ghb/repository_configurator.rb @@ -152,11 +152,13 @@ def validate_required_checks!(expected_checks, actual_checks, protection_exists) end def build_branch_protection_payload(current_protection, expected_checks, protection_exists, sync_required_status_checks) - # Preserve existing dismissal restrictions / bypass allowances or use empty defaults - dismissal_users = current_protection.dig('required_pull_request_reviews', 'dismissal_restrictions', 'users')&.map { |u| u['login'] } || [] - dismissal_teams = current_protection.dig('required_pull_request_reviews', 'dismissal_restrictions', 'teams')&.map { |t| t['slug'] } || [] - bypass_users = current_protection.dig('required_pull_request_reviews', 'bypass_pull_request_allowances', 'users')&.map { |u| u['login'] } || [] - bypass_teams = current_protection.dig('required_pull_request_reviews', 'bypass_pull_request_allowances', 'teams')&.map { |t| t['slug'] } || [] + # Preserve existing dismissal restrictions / bypass allowances or use empty defaults. + # filter_map drops entries GitHub returns without a login/slug so the PUT body + # never contains a [null] users/teams array (which GitHub rejects with 422). + dismissal_users = current_protection.dig('required_pull_request_reviews', 'dismissal_restrictions', 'users')&.filter_map { |u| u['login'] } || [] + dismissal_teams = current_protection.dig('required_pull_request_reviews', 'dismissal_restrictions', 'teams')&.filter_map { |t| t['slug'] } || [] + bypass_users = current_protection.dig('required_pull_request_reviews', 'bypass_pull_request_allowances', 'users')&.filter_map { |u| u['login'] } || [] + bypass_teams = current_protection.dig('required_pull_request_reviews', 'bypass_pull_request_allowances', 'teams')&.filter_map { |t| t['slug'] } || [] # Use existing checks if protection exists, otherwise build from expected checks. # When syncing, rebuild from expected_checks but preserve app_id from existing entries diff --git a/lib/ghb/workflow/workflow.rb b/lib/ghb/workflow/workflow.rb index f3da3b9..4a0156c 100644 --- a/lib/ghb/workflow/workflow.rb +++ b/lib/ghb/workflow/workflow.rb @@ -79,8 +79,17 @@ def read(file) # Convert github_token to github-token on load for consistency content.gsub!('github_token:', 'github-token:') - workflow_data = Psych.safe_load(content)&.deep_symbolize_keys - return if workflow_data.nil? + begin + parsed = Psych.safe_load(content) + rescue Psych::SyntaxError => e + raise(ConfigError, "Invalid YAML in #{file}: #{e.message}") + end + + return if parsed.nil? + + raise(ConfigError, "Invalid workflow file #{file}: expected a mapping at the document root, got #{parsed.class}") unless parsed.is_a?(Hash) + + workflow_data = parsed.deep_symbolize_keys @name = workflow_data[:name] @run_name = workflow_data[:'run-name'] diff --git a/spec/fixtures/workflow_generation/build.yml b/spec/fixtures/workflow_generation/build.yml new file mode 100644 index 0000000..9f74bdd --- /dev/null +++ b/spec/fixtures/workflow_generation/build.yml @@ -0,0 +1,133 @@ +# github-build --organization test-org --skip_repository_settings --skip_gitignore --skip_slack +--- +name: Build +'on': + pull_request: + types: + - opened + - edited + - reopened + - synchronize + push: + branches: + - master + - "[0-9]*" + - dependabot/** + tags: + - "**" +permissions: + contents: read + pull-requests: read +env: + RUBY-BUNDLER-CACHE: true +jobs: + variables: + name: Prepare Variables + runs-on: ubuntu-latest + outputs: + BUILD_NAME: "${{steps.variables.outputs.BUILD_NAME}}" + BUILD_VERSION: "${{steps.variables.outputs.BUILD_VERSION}}" + COMMIT_MESSAGE: "${{steps.variables.outputs.COMMIT_MESSAGE}}" + MODIFIED_GITHUB_RUN_NUMBER: "${{steps.variables.outputs.MODIFIED_GITHUB_RUN_NUMBER}}" + DEPLOY_ON_BETA: "${{steps.variables.outputs.DEPLOY_ON_BETA}}" + DEPLOY_ON_RC: "${{steps.variables.outputs.DEPLOY_ON_RC}}" + DEPLOY_ON_PROD: "${{steps.variables.outputs.DEPLOY_ON_PROD}}" + DEPLOY_MACOS: "${{steps.variables.outputs.DEPLOY_MACOS}}" + DEPLOY_TVOS: "${{steps.variables.outputs.DEPLOY_TVOS}}" + DEPLOY_OPTIONS: "${{steps.variables.outputs.DEPLOY_OPTIONS}}" + SKIP_LICENSES: "${{steps.variables.outputs.SKIP_LICENSES}}" + SKIP_LINTERS: "${{steps.variables.outputs.SKIP_LINTERS}}" + SKIP_TESTS: "${{steps.variables.outputs.SKIP_TESTS}}" + UPDATE_PACKAGES: "${{steps.variables.outputs.UPDATE_PACKAGES}}" + LINTERS: "${{steps.variables.outputs.LINTERS}}" + timeout-minutes: 30 + steps: + - name: Prepare variables + id: variables + uses: cloud-officer/ci-actions/variables@v2 + with: + ssh-key: "${{secrets.SSH_KEY}}" + github-token: "${{secrets.GH_PAT}}" + rubocop: + name: Ruby Linter + runs-on: ubuntu-latest + needs: + - variables + if: "${{needs.variables.outputs.SKIP_LINTERS != '1' && github.event_name == 'pull_request'}}" + timeout-minutes: 30 + steps: + - name: Rubocop + uses: cloud-officer/ci-actions/linters/rubocop@v2 + with: + linters: "${{needs.variables.outputs.LINTERS}}" + ssh-key: "${{secrets.SSH_KEY}}" + github-token: "${{secrets.GH_PAT}}" + semgrep: + name: Semgrep Security Scanner + runs-on: ubuntu-latest + needs: + - variables + if: "${{needs.variables.outputs.SKIP_LINTERS != '1' && github.event_name == 'pull_request'}}" + timeout-minutes: 30 + steps: + - name: Semgrep + uses: cloud-officer/ci-actions/linters/semgrep@v2 + with: + linters: "${{needs.variables.outputs.LINTERS}}" + ssh-key: "${{secrets.SSH_KEY}}" + github-token: "${{secrets.GH_PAT}}" + yamllint: + name: YAML Linter + runs-on: ubuntu-latest + needs: + - variables + if: "${{needs.variables.outputs.SKIP_LINTERS != '1' && github.event_name == 'pull_request'}}" + timeout-minutes: 30 + steps: + - name: Yamllint + uses: cloud-officer/ci-actions/linters/yamllint@v2 + with: + linters: "${{needs.variables.outputs.LINTERS}}" + ssh-key: "${{secrets.SSH_KEY}}" + github-token: "${{secrets.GH_PAT}}" + licenses: + name: Licenses Check + runs-on: ubuntu-latest + needs: + - variables + if: "${{needs.variables.outputs.SKIP_LICENSES != '1'}}" + timeout-minutes: 30 + steps: + - name: Licenses + uses: cloud-officer/ci-actions/soup@v2 + with: + ssh-key: "${{secrets.SSH_KEY}}" + github-token: "${{secrets.GH_PAT}}" + parameters: "--no_prompt" + ruby_unit_tests: + name: Ruby Unit Tests + runs-on: ubuntu-latest + needs: + - variables + if: "${{needs.variables.outputs.SKIP_TESTS != '1'}}" + timeout-minutes: 30 + steps: + - name: Setup + uses: cloud-officer/ci-actions/setup@v2 + with: + ssh-key: "${{secrets.SSH_KEY}}" + github-token: "${{secrets.GH_PAT}}" + aws-access-key-id: "${{secrets.AWS_ACCESS_KEY_ID}}" + aws-secret-access-key: "${{secrets.AWS_SECRET_ACCESS_KEY}}" + aws-region: "${{secrets.AWS_DEFAULT_REGION}}" + ruby-bundler-cache: "${{env.RUBY-BUNDLER-CACHE}}" + - name: Bundler + shell: bash + run: bundle install + env: + GITHUB_TOKEN: "${{secrets.GH_PAT}}" + - name: RSpec + shell: bash + run: bundle exec rspec + env: + GITHUB_TOKEN: "${{secrets.GH_PAT}}" diff --git a/spec/ghb/application_spec.rb b/spec/ghb/application_spec.rb index b755517..fd11585 100644 --- a/spec/ghb/application_spec.rb +++ b/spec/ghb/application_spec.rb @@ -212,4 +212,52 @@ def validate_config! .to(raise_error(GHB::ConfigError, 'Missing required linters config file: custom/path/linters.yaml')) end end + + describe 'private internals' do + let(:internals_class) do + Class.new(described_class) do + def initialize; end # rubocop:disable Lint/MissingSuper + + public :detect_default_branch, :validate_entries + end + end + let(:app) { internals_class.new } + + describe '#detect_default_branch' do + it 'returns the branch reported by git symbolic-ref' do + allow(app).to(receive(:`).with('git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null').and_return("refs/remotes/origin/main\n")) + + expect(app.detect_default_branch).to(eq('main')) + end + + it "falls back to 'master' when origin/HEAD is not resolvable" do + allow(app).to(receive(:`).with('git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null').and_return('')) + + expect(app.detect_default_branch).to(eq('master')) + end + + it "detects 'master' when that is the default branch" do + allow(app).to(receive(:`).with('git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null').and_return("refs/remotes/origin/master\n")) + + expect(app.detect_default_branch).to(eq('master')) + end + end + + describe '#validate_entries' do + it 'is permissive: silently skips entry values that are not a Hash' do + expect { app.validate_entries({ rubocop: 'true' }, 'config/linters.yaml', 'linter', %w[short_name]) } + .not_to(raise_error) + end + + it 'returns without error when the document root is not a Hash' do + expect { app.validate_entries([], 'config/linters.yaml', 'linter', %w[short_name]) } + .not_to(raise_error) + end + + it 'still raises for a Hash entry missing required keys (skip is value-type only)' do + expect { app.validate_entries({ bad: { short_name: 'x' } }, 'config/linters.yaml', 'linter', %w[short_name long_name]) } + .to(raise_error(GHB::ConfigError, %r{Linter 'bad' in config/linters.yaml is missing required keys: long_name})) + end + end + end end diff --git a/spec/ghb/git_hub_api_client_spec.rb b/spec/ghb/git_hub_api_client_spec.rb index ffe8f26..284a7f0 100644 --- a/spec/ghb/git_hub_api_client_spec.rb +++ b/spec/ghb/git_hub_api_client_spec.rb @@ -188,6 +188,20 @@ expect(response.code).to(eq(200)) end + it 'sleeps with a linear back-off (1s, 2s, 3s) between 5xx retries' do # rubocop:disable RSpec/ExampleLength,RSpec/MultipleExpectations + stub_request(:get, base_url) + .to_return(status: 503, body: '{}') + .then.to_return(status: 503, body: '{}') + .then.to_return(status: 503, body: '{}') + .then.to_return(status: 200, body: '{"ok":true}') + + client.get(base_url) + + expect(client).to(have_received(:sleep).with(1).ordered) + expect(client).to(have_received(:sleep).with(2).ordered) + expect(client).to(have_received(:sleep).with(3).ordered) + end + it 'raises after exhausting retries on 5xx' do stub_request(:get, base_url) .to_return(status: 503, body: '{}') diff --git a/spec/ghb/gitignore_manager_spec.rb b/spec/ghb/gitignore_manager_spec.rb index 5c3f99a..37f4b16 100644 --- a/spec/ghb/gitignore_manager_spec.rb +++ b/spec/ghb/gitignore_manager_spec.rb @@ -123,6 +123,36 @@ expect { manager.update } .to(raise_error(RuntimeError, /Cannot fetch gitignore templates/)) end + + [503, 504].each do |status| + it "raises a clear error on a #{status} response" do # rubocop:disable RSpec/ExampleLength + config_yaml = Psych.dump(minimal_gitignore_config.deep_stringify_keys) + + allow(manager).to(receive_messages(cached_file_read: config_yaml, find_files_matching: [])) + allow(File).to(receive(:exist?).with('.gitignore').and_return(false)) + allow(File).to(receive(:exist?).with(anything).and_return(false)) + + api_response = double('HTTParty::Response', code: status, message: 'Service Unavailable') # rubocop:disable RSpec/VerifiedDoubles + allow(HTTParty).to(receive(:get).with(anything, timeout: 30).and_return(api_response)) + + expect { manager.update } + .to(raise_error(RuntimeError, /Cannot fetch gitignore templates/)) + end + end + + [Net::OpenTimeout, Net::ReadTimeout, Errno::ECONNRESET, SocketError].each do |error| + it "wraps a transient #{error} into an actionable error instead of a raw stack trace" do # rubocop:disable RSpec/ExampleLength + config_yaml = Psych.dump(minimal_gitignore_config.deep_stringify_keys) + + allow(manager).to(receive_messages(cached_file_read: config_yaml, find_files_matching: [])) + allow(File).to(receive(:exist?).with('.gitignore').and_return(false)) + allow(File).to(receive(:exist?).with(anything).and_return(false)) + allow(HTTParty).to(receive(:get).with(anything, timeout: 30).and_raise(error)) + + expect { manager.update } + .to(raise_error(RuntimeError, /Cannot fetch gitignore templates: #{error}/)) + end + end end describe '#detect_gitignore_templates (private)' do diff --git a/spec/ghb/integration/workflow_generation_spec.rb b/spec/ghb/integration/workflow_generation_spec.rb new file mode 100644 index 0000000..bd614e7 --- /dev/null +++ b/spec/ghb/integration/workflow_generation_spec.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +# Golden-file (snapshot) test for the end-to-end workflow generation. +# +# Feeds a minimal Ruby project tree to Application#execute inside an isolated +# tmpdir and diffs the generated .github/workflows/build.yml against a +# checked-in expected file. This catches structural drift (wrong needs: graph, +# missing permissions:, mis-ordered jobs/steps) that substring assertions miss. +# +# Regenerate the golden file after an intentional change with: +# UPDATE_SNAPSHOTS=1 bundle exec rspec spec/ghb/integration/workflow_generation_spec.rb +RSpec.describe('workflow generation (golden file)') do # rubocop:disable RSpec/DescribeClass + let(:golden_path) { "#{__dir__}/../../fixtures/workflow_generation/build.yml" } + let(:argv) do + %w[--organization test-org --skip_repository_settings --skip_gitignore --skip_slack] + end + + around do |example| + Dir.mktmpdir('ghb-golden') do |dir| + Dir.chdir(dir) { example.run } # rubocop:disable ThreadSafety/DirChdir + end + end + + before do + # Minimal Ruby project so the Ruby language + linters are detected. + # .ruby-version matches config/languages.yaml ruby-version so the + # version-file reconciliation neither warns nor rewrites mid-test. + File.write('app.rb', "puts 'hello'\n") + File.write('Gemfile', "source 'https://rubygems.org'\n") + File.write('.ruby-version', "4.0.4\n") + allow($stdout).to(receive(:puts)) + end + + it 'generates build.yml matching the checked-in golden file' do # rubocop:disable RSpec/ExampleLength,RSpec/MultipleExpectations + exit_code = GHB::Application.new(argv).execute + expect(exit_code).to(eq(GHB::Status::SUCCESS_EXIT_CODE)) + + generated = File.read('.github/workflows/build.yml') + + if ENV['UPDATE_SNAPSHOTS'] + FileUtils.mkdir_p(File.dirname(golden_path)) + File.write(golden_path, generated) + skip("Golden file updated: #{golden_path}") # rubocop:disable RSpec/Pending + end + + raise("Missing golden file. Run with UPDATE_SNAPSHOTS=1 to create #{golden_path}") unless File.exist?(golden_path) + + expect(generated).to(eq(File.read(golden_path))) + end +end diff --git a/spec/ghb/options_spec.rb b/spec/ghb/options_spec.rb index 0578de3..9fc5e85 100644 --- a/spec/ghb/options_spec.rb +++ b/spec/ghb/options_spec.rb @@ -303,33 +303,33 @@ end end - describe 'args_from_file (private)' do + describe 'args from build file' do it 'returns empty array when file does not exist' do allow(File).to(receive(:exist?).and_return(false)) options = described_class.new([]) - expect(options.instance_variable_get(:@argv)).to(eq([])) + expect(options.original_argv).to(eq([])) end it 'returns empty array when first line does not start with prefix' do allow(File).to(receive_messages(exist?: true, foreach: ["name: CI\n", "on: push\n"].each)) options = described_class.new([]) - expect(options.instance_variable_get(:@argv)).to(eq([])) + expect(options.original_argv).to(eq([])) end it 'handles empty file gracefully' do allow(File).to(receive_messages(exist?: true, foreach: [].each)) options = described_class.new([]) - expect(options.instance_variable_get(:@argv)).to(eq([])) + expect(options.original_argv).to(eq([])) end it 'parses arguments with shellwords' do allow(File).to(receive_messages(exist?: true, foreach: ["# github-build --organization 'My Org' --skip_slack\n"].each)) options = described_class.new([]) - expect(options.instance_variable_get(:@argv)).to(eq(['--organization', 'My Org', '--skip_slack'])) + expect(options.original_argv).to(eq(['--organization', 'My Org', '--skip_slack'])) end it 'raises a clear ConfigError on malformed quoting instead of a raw Shellwords stack trace' do diff --git a/spec/ghb/repository_configurator_spec.rb b/spec/ghb/repository_configurator_spec.rb index 8d7125e..e07e3bc 100644 --- a/spec/ghb/repository_configurator_spec.rb +++ b/spec/ghb/repository_configurator_spec.rb @@ -355,6 +355,81 @@ end end + context 'when GitHub returns dismissal/bypass users without a login key' do # rubocop:disable RSpec/MultipleMemoizedHelpers + let(:current_protection) do + { + required_status_checks: { + contexts: %w[Build Lint], + checks: [] + }, + required_pull_request_reviews: { + dismissal_restrictions: { + users: [{}], + teams: [{}] + }, + bypass_pull_request_allowances: { + users: [{ login: 'bot-user' }, {}], + teams: [] + } + } + } + end + + let(:repo_info_response) do + instance_double(HTTParty::Response, code: 200, body: { private: false }.to_json) + end + + let(:protection_response) do + instance_double(HTTParty::Response, code: 200, body: current_protection.to_json) + end + + let(:codeql_default_setup_response) do + instance_double(HTTParty::Response, code: 200, body: { state: 'configured', languages: %w[ruby] }.to_json) + end + + let(:codeql_get_response) do + instance_double(HTTParty::Response, code: 200, body: { state: 'configured' }.to_json) + end + + let(:ok_response) do + instance_double(HTTParty::Response, code: 200, body: '{}') + end + + before do + allow(github_client).to(receive(:get).with(repo_url).and_return(repo_info_response)) + allow(github_client).to(receive(:get).with("#{repo_url}/branches/#{default_branch}/protection", expected_codes: [200, 404]).and_return(protection_response)) + allow(github_client).to(receive(:get).with("#{repo_url}/code-scanning/default-setup", expected_codes: nil).and_return(codeql_default_setup_response)) + allow(github_client).to(receive(:put).with("#{repo_url}/branches/#{default_branch}/protection", body: anything).and_return(ok_response)) + allow(github_client).to(receive(:post).with("#{repo_url}/branches/#{default_branch}/protection/required_signatures", expected_codes: [200, 204]).and_return(ok_response)) + allow(github_client).to(receive(:put).with("#{repo_url}/vulnerability-alerts", expected_codes: [200, 204]).and_return(ok_response)) + allow(github_client).to(receive(:put).with("#{repo_url}/automated-security-fixes", expected_codes: [200, 204]).and_return(ok_response)) + allow(github_client).to(receive(:patch).with(repo_url, body: anything).and_return(ok_response)) + allow(github_client).to(receive(:get).with("#{repo_url}/code-scanning/default-setup").and_return(codeql_get_response)) + end + + it 'compacts malformed entries so the PUT body never contains a [null] list' do # rubocop:disable RSpec/ExampleLength + configurator.configure + + expect(github_client).to( + have_received(:put).with( + "#{repo_url}/branches/#{default_branch}/protection", + body: hash_including( + required_pull_request_reviews: hash_including( + dismissal_restrictions: { + users: [], + teams: [] + }, + bypass_pull_request_allowances: { + users: ['bot-user'], + teams: [] + } + ) + ) + ) + ) + end + end + context 'when branch protection exists with mismatching checks' do # rubocop:disable RSpec/MultipleMemoizedHelpers let(:current_protection) do { diff --git a/spec/ghb/workflow/workflow_spec.rb b/spec/ghb/workflow/workflow_spec.rb index d9b786f..ae27b39 100644 --- a/spec/ghb/workflow/workflow_spec.rb +++ b/spec/ghb/workflow/workflow_spec.rb @@ -245,6 +245,20 @@ expect(workflow.name).to(eq('CI')) expect(workflow.jobs).to(eq({})) end + + it 'raises a clear ConfigError on malformed YAML instead of a raw Psych::SyntaxError' do + allow(File).to(receive(:read).and_return("name: 'unterminated\n".dup)) + + expect { workflow.read('broken.yml') } + .to(raise_error(GHB::ConfigError, /Invalid YAML in broken\.yml/)) + end + + it 'raises a clear ConfigError when the document root is not a mapping' do + allow(File).to(receive(:read).and_return("---\n- 1\n- 2\n".dup)) + + expect { workflow.read('list.yml') } + .to(raise_error(GHB::ConfigError, /expected a mapping at the document root, got Array/)) + end end describe '#write' do