From d2ff50be50fadba5a80ff7d7b68439c148ebb24a Mon Sep 17 00:00:00 2001 From: Sourabh Sarvotham Parkala Date: Thu, 23 Dec 2021 14:21:35 +0100 Subject: [PATCH] Implement experimental Bandit scan check Fixes #729 --- .../experimental/BanditDataProvider.java | 190 ++++++++++++++++++ .../model/feature/oss/OssFeatures.java | 16 ++ .../data/github/CodeqlDataProviderTest.java | 2 +- .../experimental/BanditDataProviderTest.java | 136 +++++++++++++ .../bandit-analysis-with-multiple-jobs.yml | 25 +++ ...sis-with-no-bandit-run-but-uses-bandit.yml | 27 +++ .../bandit-analysis-with-no-bandit-run.yml | 25 +++ .../experimental/bandit-analysis-with-run.yml | 25 +++ 8 files changed, 445 insertions(+), 1 deletion(-) create mode 100644 src/main/java/com/sap/oss/phosphor/fosstars/data/github/experimental/BanditDataProvider.java create mode 100644 src/test/java/com/sap/oss/phosphor/fosstars/data/github/experimental/BanditDataProviderTest.java create mode 100644 src/test/resources/com/sap/oss/phosphor/fosstars/data/github/experimental/bandit-analysis-with-multiple-jobs.yml create mode 100644 src/test/resources/com/sap/oss/phosphor/fosstars/data/github/experimental/bandit-analysis-with-no-bandit-run-but-uses-bandit.yml create mode 100644 src/test/resources/com/sap/oss/phosphor/fosstars/data/github/experimental/bandit-analysis-with-no-bandit-run.yml create mode 100644 src/test/resources/com/sap/oss/phosphor/fosstars/data/github/experimental/bandit-analysis-with-run.yml diff --git a/src/main/java/com/sap/oss/phosphor/fosstars/data/github/experimental/BanditDataProvider.java b/src/main/java/com/sap/oss/phosphor/fosstars/data/github/experimental/BanditDataProvider.java new file mode 100644 index 000000000..e5dcbc46e --- /dev/null +++ b/src/main/java/com/sap/oss/phosphor/fosstars/data/github/experimental/BanditDataProvider.java @@ -0,0 +1,190 @@ +package com.sap.oss.phosphor.fosstars.data.github.experimental; + +import static com.sap.oss.phosphor.fosstars.model.feature.oss.OssFeatures.RUNS_BANDIT_SCANS; +import static com.sap.oss.phosphor.fosstars.model.feature.oss.OssFeatures.USES_BANDIT_SCAN_CHECKS; +import static com.sap.oss.phosphor.fosstars.model.other.Utils.setOf; + +import com.sap.oss.phosphor.fosstars.data.github.GitHubCachingDataProvider; +import com.sap.oss.phosphor.fosstars.data.github.GitHubDataFetcher; +import com.sap.oss.phosphor.fosstars.data.github.LocalRepository; +import com.sap.oss.phosphor.fosstars.model.Feature; +import com.sap.oss.phosphor.fosstars.model.Value; +import com.sap.oss.phosphor.fosstars.model.ValueSet; +import com.sap.oss.phosphor.fosstars.model.feature.oss.OssFeatures; +import com.sap.oss.phosphor.fosstars.model.subject.oss.GitHubProject; +import com.sap.oss.phosphor.fosstars.model.value.ValueHashSet; +import com.sap.oss.phosphor.fosstars.util.Yaml; +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.regex.Pattern; +import org.apache.commons.collections4.IteratorUtils; + +/** + * The data provider gathers info about how a project uses Bandit for static analysis. In + * particular, it tries to fill out the following features: + * + */ +public class BanditDataProvider extends GitHubCachingDataProvider { + + /** + * A directory where GitHub Actions configs are stored. + */ + private static final String GITHUB_ACTIONS_DIRECTORY = ".github/workflows"; + + /** + * A list of extensions of GitHub Actions configs. + */ + private static final List GITHUB_ACTIONS_CONFIG_EXTENSIONS + = Arrays.asList(".yaml", ".yml"); + + /** + * A step in a GitHub action that triggers analysis with CodeQL. + */ + private static final Pattern RUN_STEP_BANDIT_REGEX_PATTERN + = Pattern.compile("^.*bandit .*$", Pattern.DOTALL); + + /** + * Initializes a data provider. + * + * @param fetcher An interface to GitHub. + */ + public BanditDataProvider(GitHubDataFetcher fetcher) { + super(fetcher); + } + + @Override + public Set> supportedFeatures() { + return setOf(RUNS_BANDIT_SCANS, USES_BANDIT_SCAN_CHECKS); + } + + @Override + protected ValueSet fetchValuesFor(GitHubProject project) throws IOException { + logger.info("Figuring out how the project uses Bandit ..."); + + LocalRepository repository = GitHubDataFetcher.localRepositoryFor(project); + + Value runsBandit = RUNS_BANDIT_SCANS.value(false); + Value usesBanditScanChecks = USES_BANDIT_SCAN_CHECKS.value(false); + + // ideally, we're looking for a GitHub action that runs Bandit scan on pull requests + // but if we just find an action that runs Bandit scans, that's also fine + for (Path configPath : findGitHubActionsIn(repository)) { + try (InputStream content = Files.newInputStream(configPath)) { + Map githubAction = Yaml.readMap(content); + if (triggersBanditScan(githubAction)) { + runsBandit = RUNS_BANDIT_SCANS.value(true); + if (runsOnPullRequests(githubAction)) { + usesBanditScanChecks = USES_BANDIT_SCAN_CHECKS.value(true); + break; + } + } + } + } + + return ValueHashSet.from(runsBandit, usesBanditScanChecks); + } + + /** + * Looks for GitHub actions in a repository. + * + * @param repository The repository to be checked. + * @return A list of paths to GitHub Action configs. + * @throws IOException If something went wrong. + */ + private static List findGitHubActionsIn(LocalRepository repository) throws IOException { + Path path = Paths.get(GITHUB_ACTIONS_DIRECTORY); + + if (!repository.hasDirectory(path)) { + return Collections.emptyList(); + } + + return repository.files(path, BanditDataProvider::isGitHubActionConfig); + } + + /** + * Checks if a GitHub action triggers a Bandit scan. + * + * @param githubAction A config for the action. + * @return True if the action triggers a Bandit scan, false otherwise. + */ + private static boolean triggersBanditScan(Map githubAction) { + return Optional.ofNullable(githubAction.get("jobs")) + .filter(Map.class::isInstance) + .map(Map.class::cast) + .map(jobs -> jobs.values()) + .filter(Iterable.class::isInstance) + .map(Iterable.class::cast) + .map(BanditDataProvider::scanJobs) + .orElse(false); + } + + /** + * Checks if any step in a collection of jobs triggers a Bandit scan. + * + * @param jobs The collection of jobs from GitHub action. + * @return True if a step triggers a Bandit scan, false otherwise. + */ + private static boolean scanJobs(Iterable jobs) { + return IteratorUtils.toList(jobs.iterator()).stream() + .filter(Map.class::isInstance) + .map(Map.class::cast) + .map(job -> job.get("steps")) + .filter(Iterable.class::isInstance) + .map(Iterable.class::cast) + .anyMatch(BanditDataProvider::hasBanditRunStep); + } + + /** + * Checks if a collection of steps from a GitHub action contains a step that triggers a Bandit + * scan. + * + * @param steps The steps to be checked. + * @return True if the steps contain a step that triggers a Bandit scan, false otherwise. + */ + private static boolean hasBanditRunStep(Iterable steps) { + return IteratorUtils.toList(steps.iterator()).stream() + .filter(Map.class::isInstance) + .map(Map.class::cast) + .map(step -> step.get("run")) + .filter(String.class::isInstance) + .map(String.class::cast) + .anyMatch(run -> RUN_STEP_BANDIT_REGEX_PATTERN.matcher(run).matches()); + } + + /** + * Checks if a GitHub action runs on pull requests. + * + * @param githubAction A config of the action. + * @return True if the action runs on pull requests, false otherwise. + */ + private static boolean runsOnPullRequests(Map githubAction) { + return Optional.ofNullable(githubAction.get("on")) + .filter(Map.class::isInstance) + .map(Map.class::cast) + .map(on -> on.containsKey("pull_request")) + .orElse(false); + } + + /** + * Checks if a file is a config for a GitHub action. + * + * @param path A path to the file. + * @return True if a file looks like a config for a GitHub action, false otherwise. + */ + private static boolean isGitHubActionConfig(Path path) { + return GITHUB_ACTIONS_CONFIG_EXTENSIONS + .stream().anyMatch(ext -> path.getFileName().toString().endsWith(ext)); + } +} \ No newline at end of file diff --git a/src/main/java/com/sap/oss/phosphor/fosstars/model/feature/oss/OssFeatures.java b/src/main/java/com/sap/oss/phosphor/fosstars/model/feature/oss/OssFeatures.java index f36d0f120..970de9cbf 100755 --- a/src/main/java/com/sap/oss/phosphor/fosstars/model/feature/oss/OssFeatures.java +++ b/src/main/java/com/sap/oss/phosphor/fosstars/model/feature/oss/OssFeatures.java @@ -222,6 +222,22 @@ private OssFeatures() { public static final Feature USES_CODEQL_CHECKS = new BooleanFeature("If a project runs CodeQL checks for commits"); + /** + * Shows if an open-source project runs Bandit scans. + * + * @see Enabling code scanning for a repository + */ + public static final Feature RUNS_BANDIT_SCANS + = new BooleanFeature("If a project runs Bandit scans"); + + /** + * Shows if an open-source project runs Bandit checks for commits. + * + * @see Enabling code scanning for a repository + */ + public static final Feature USES_BANDIT_SCAN_CHECKS + = new BooleanFeature("If a project runs Bandit scan checks for commits"); + /** * Shows if an open-source project uses LGTM checks for commits. */ diff --git a/src/test/java/com/sap/oss/phosphor/fosstars/data/github/CodeqlDataProviderTest.java b/src/test/java/com/sap/oss/phosphor/fosstars/data/github/CodeqlDataProviderTest.java index 6b159a0c2..5d4c8b197 100644 --- a/src/test/java/com/sap/oss/phosphor/fosstars/data/github/CodeqlDataProviderTest.java +++ b/src/test/java/com/sap/oss/phosphor/fosstars/data/github/CodeqlDataProviderTest.java @@ -97,7 +97,7 @@ private void testCodeqlRuns(String filename, InputStream content, Value... ex CodeqlDataProvider provider = new CodeqlDataProvider(fetcher); ValueSet values = provider.fetchValuesFor(PROJECT); - assertEquals(2, expectedValues.length); + assertEquals(2, values.size()); for (Value expectedValue : expectedValues) { Optional> something = values.of(expectedValue.feature()); assertTrue(something.isPresent()); diff --git a/src/test/java/com/sap/oss/phosphor/fosstars/data/github/experimental/BanditDataProviderTest.java b/src/test/java/com/sap/oss/phosphor/fosstars/data/github/experimental/BanditDataProviderTest.java new file mode 100644 index 000000000..b735d69e9 --- /dev/null +++ b/src/test/java/com/sap/oss/phosphor/fosstars/data/github/experimental/BanditDataProviderTest.java @@ -0,0 +1,136 @@ +package com.sap.oss.phosphor.fosstars.data.github.experimental; + +import static com.sap.oss.phosphor.fosstars.model.feature.oss.OssFeatures.RUNS_BANDIT_SCANS; +import static com.sap.oss.phosphor.fosstars.model.feature.oss.OssFeatures.USES_BANDIT_SCAN_CHECKS; +import static org.hamcrest.CoreMatchers.hasItem; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.sap.oss.phosphor.fosstars.data.github.LocalRepository; +import com.sap.oss.phosphor.fosstars.data.github.PackageManagementTest; +import com.sap.oss.phosphor.fosstars.data.github.TestGitHubDataFetcherHolder; +import com.sap.oss.phosphor.fosstars.model.Feature; +import com.sap.oss.phosphor.fosstars.model.Value; +import com.sap.oss.phosphor.fosstars.model.ValueSet; +import com.sap.oss.phosphor.fosstars.model.subject.oss.GitHubProject; +import java.io.IOException; +import java.io.InputStream; +import java.io.UncheckedIOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Collections; +import java.util.Optional; +import java.util.Set; +import org.apache.commons.io.FileUtils; +import org.apache.commons.io.IOUtils; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +public class BanditDataProviderTest extends TestGitHubDataFetcherHolder { + + private static final GitHubProject PROJECT = new GitHubProject("org", "test"); + + private static final String GITHUB_WORKFLOW_FILENAME = ".github/workflows/bandit.yml"; + + private static Path repositoryDirectory; + + private static LocalRepository localRepository; + + @BeforeClass + public static void setup() { + try { + repositoryDirectory = Files.createTempDirectory(PackageManagementTest.class.getName()); + localRepository = mock(LocalRepository.class); + TestGitHubDataFetcher.addForTesting(PROJECT, localRepository); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + @Test + public void testNotInteractive() { + assertFalse(new BanditDataProvider(fetcher).interactive()); + } + + @Test + public void testSupportedFeatures() { + Set> features = new BanditDataProvider(fetcher).supportedFeatures(); + assertEquals(2, features.size()); + assertThat(features, hasItem(RUNS_BANDIT_SCANS)); + assertThat(features, hasItem(USES_BANDIT_SCAN_CHECKS)); + } + + @Test + public void testWithBanditRunsAndChecks() throws IOException { + try (InputStream content = getClass().getResourceAsStream("bandit-analysis-with-run.yml")) { + testBanditRuns(GITHUB_WORKFLOW_FILENAME, content, + RUNS_BANDIT_SCANS.value(true), + USES_BANDIT_SCAN_CHECKS.value(true)); + } + } + + @Test + public void testWithBanditRunsAndMultipleJobs() throws IOException { + try (InputStream content = getClass().getResourceAsStream( + "bandit-analysis-with-multiple-jobs.yml")) { + testBanditRuns(GITHUB_WORKFLOW_FILENAME, content, + RUNS_BANDIT_SCANS.value(true), + USES_BANDIT_SCAN_CHECKS.value(false)); + } + } + + @Test + public void testWithNoBanditRunsButInstallBandit() throws IOException { + try (InputStream content = getClass().getResourceAsStream( + "bandit-analysis-with-no-bandit-run.yml")) { + testBanditRuns(GITHUB_WORKFLOW_FILENAME, content, + RUNS_BANDIT_SCANS.value(false), + USES_BANDIT_SCAN_CHECKS.value(false)); + } + } + + @Test + public void testWithNoBanditRunsButInstallsBanditAndUsesBandit() throws IOException { + try (InputStream content = getClass().getResourceAsStream( + "bandit-analysis-with-no-bandit-run-but-uses-bandit.yml")) { + testBanditRuns(GITHUB_WORKFLOW_FILENAME, content, + RUNS_BANDIT_SCANS.value(false), + USES_BANDIT_SCAN_CHECKS.value(false)); + } + } + + private void testBanditRuns(String filename, InputStream content, Value... expectedValues) + throws IOException { + + Path file = repositoryDirectory.resolve(filename); + Files.createDirectories(file.getParent()); + when(localRepository.hasDirectory(any(Path.class))).thenReturn(true); + IOUtils.copy(content, Files.newOutputStream(file)); + when(localRepository.files(any(), any())).thenReturn(Collections.singletonList(file)); + + BanditDataProvider provider = new BanditDataProvider(fetcher); + ValueSet values = provider.fetchValuesFor(PROJECT); + + assertEquals(2, values.size()); + for (Value expectedValue : expectedValues) { + Optional> something = values.of(expectedValue.feature()); + assertTrue(something.isPresent()); + assertEquals(expectedValue, something.get()); + } + } + + @AfterClass + public static void shutdown() { + try { + FileUtils.forceDeleteOnExit(repositoryDirectory.toFile()); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } +} \ No newline at end of file diff --git a/src/test/resources/com/sap/oss/phosphor/fosstars/data/github/experimental/bandit-analysis-with-multiple-jobs.yml b/src/test/resources/com/sap/oss/phosphor/fosstars/data/github/experimental/bandit-analysis-with-multiple-jobs.yml new file mode 100644 index 000000000..d5a00a47a --- /dev/null +++ b/src/test/resources/com/sap/oss/phosphor/fosstars/data/github/experimental/bandit-analysis-with-multiple-jobs.yml @@ -0,0 +1,25 @@ +name: "Bandit" +on: + push: + branches: [master] + schedule: + - cron: '0 13 * * 3' +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + + - name: Use Python + uses: actions/setup-python@v2 + with: + python-version: '3.x' + architecture: 'x64' + bandit: + steps: + - run: | + python -m pip install --upgrade pip + pip install -r requirements.txt + - run: | + mkdir -p reports + bandit --format json --output reports/bandit-report.json --recursive test \ No newline at end of file diff --git a/src/test/resources/com/sap/oss/phosphor/fosstars/data/github/experimental/bandit-analysis-with-no-bandit-run-but-uses-bandit.yml b/src/test/resources/com/sap/oss/phosphor/fosstars/data/github/experimental/bandit-analysis-with-no-bandit-run-but-uses-bandit.yml new file mode 100644 index 000000000..af2531621 --- /dev/null +++ b/src/test/resources/com/sap/oss/phosphor/fosstars/data/github/experimental/bandit-analysis-with-no-bandit-run-but-uses-bandit.yml @@ -0,0 +1,27 @@ +name: "Bandit" +on: + push: + branches: [master] + pull_request: + branches: [ master ] + schedule: + - cron: '0 13 * * 3' +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: Use Python + uses: actions/setup-python@v2 + with: + python-version: '3.x' + architecture: 'x64' + - name: Install bandit + run: pip install bandit + bandit: + steps: + - run: | + python -m pip install --upgrade pip + pip install -r requirements.txt + - run: | + mkdir -p reports \ No newline at end of file diff --git a/src/test/resources/com/sap/oss/phosphor/fosstars/data/github/experimental/bandit-analysis-with-no-bandit-run.yml b/src/test/resources/com/sap/oss/phosphor/fosstars/data/github/experimental/bandit-analysis-with-no-bandit-run.yml new file mode 100644 index 000000000..5ea2af6a1 --- /dev/null +++ b/src/test/resources/com/sap/oss/phosphor/fosstars/data/github/experimental/bandit-analysis-with-no-bandit-run.yml @@ -0,0 +1,25 @@ +name: "Bandit" +on: + push: + branches: [master] + schedule: + - cron: '0 13 * * 3' +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: Use Python + uses: actions/setup-python@v2 + with: + python-version: '3.x' + architecture: 'x64' + - name: Install bandit + run: pip install bandit + bandit: + steps: + - run: | + python -m pip install --upgrade pip + pip install -r requirements.txt + - run: | + mkdir -p reports \ No newline at end of file diff --git a/src/test/resources/com/sap/oss/phosphor/fosstars/data/github/experimental/bandit-analysis-with-run.yml b/src/test/resources/com/sap/oss/phosphor/fosstars/data/github/experimental/bandit-analysis-with-run.yml new file mode 100644 index 000000000..739b4e708 --- /dev/null +++ b/src/test/resources/com/sap/oss/phosphor/fosstars/data/github/experimental/bandit-analysis-with-run.yml @@ -0,0 +1,25 @@ +name: "Bandit" +on: + push: + branches: [master] + pull_request: + branches: [master] + schedule: + - cron: '0 13 * * 3' +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + + - name: Use Python + uses: actions/setup-python@v2 + with: + python-version: '3.x' + architecture: 'x64' + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install -r requirements.txt + - name: Run Bandit (Python code checker) + run: bandit -r . -f xml -o bandit.xml || true \ No newline at end of file