diff --git a/core/src/main/java/jenkins/widgets/HistoryPageFilter.java b/core/src/main/java/jenkins/widgets/HistoryPageFilter.java index adc09b12f22d..cc64f7b3630a 100644 --- a/core/src/main/java/jenkins/widgets/HistoryPageFilter.java +++ b/core/src/main/java/jenkins/widgets/HistoryPageFilter.java @@ -42,6 +42,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Set; import jenkins.model.queue.QueueItem; /** @@ -371,8 +372,9 @@ private boolean fitsSearchString(Object data) { private boolean fitsSearchBuildVariables(AbstractBuild runAsBuild) { Map buildVariables = runAsBuild.getBuildVariables(); - for (String paramsValues : buildVariables.values()) { - if (fitsSearchString(paramsValues)) { + Set sensitiveBuildVariables = runAsBuild.getSensitiveBuildVariables(); + for (Map.Entry param : buildVariables.entrySet()) { + if (!sensitiveBuildVariables.contains(param.getKey()) && fitsSearchString(param.getValue())) { return true; } } diff --git a/core/src/test/java/jenkins/widgets/HistoryPageFilterTest.java b/core/src/test/java/jenkins/widgets/HistoryPageFilterTest.java index 34f7800d33fb..8739093c606c 100644 --- a/core/src/test/java/jenkins/widgets/HistoryPageFilterTest.java +++ b/core/src/test/java/jenkins/widgets/HistoryPageFilterTest.java @@ -43,6 +43,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; import jenkins.model.queue.QueueItem; import org.junit.Assert; @@ -485,6 +486,7 @@ private static class MockBuild extends Build { private final int buildNumber; private Map buildVariables = Collections.emptyMap(); + private Set sensitiveBuildVariables = Collections.emptySet(); private MockBuild(int buildNumber) { super(Mockito.mock(FreeStyleProject.class), Mockito.mock(Calendar.class)); @@ -501,6 +503,11 @@ public Map getBuildVariables() { return buildVariables; } + @Override + public Set getSensitiveBuildVariables() { + return sensitiveBuildVariables; // TODO This is never actually set (bad Mock), actual test in test harness + } + MockBuild withBuildVariables(Map buildVariables) { this.buildVariables = buildVariables; return this; diff --git a/test/src/test/java/jenkins/widgets/HistoryPageFilterTest.java b/test/src/test/java/jenkins/widgets/HistoryPageFilterTest.java new file mode 100644 index 000000000000..3dc26fbcaf32 --- /dev/null +++ b/test/src/test/java/jenkins/widgets/HistoryPageFilterTest.java @@ -0,0 +1,122 @@ +package jenkins.widgets; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.empty; + +import hudson.model.AbstractBuild; +import hudson.model.FreeStyleBuild; +import hudson.model.FreeStyleProject; +import hudson.model.ParametersAction; +import hudson.model.ParametersDefinitionProperty; +import hudson.model.PasswordParameterDefinition; +import hudson.model.StringParameterDefinition; +import hudson.tasks.BuildWrapper; +import hudson.util.Secret; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.JenkinsRule; + + +public class HistoryPageFilterTest { + @Rule + public JenkinsRule j = new JenkinsRule(); + + @Test + public void doNotFindSensitiveBuildParams() throws Exception { + final FreeStyleProject freeStyleProject = j.createFreeStyleProject(); + final PasswordParameterDefinition passwordParameterDefinition = new PasswordParameterDefinition("password", Secret.fromString("t0ps3cr3t"), "description"); + final StringParameterDefinition stringParameterDefinition = new StringParameterDefinition("key", "value", "desc"); + freeStyleProject.addProperty(new ParametersDefinitionProperty(passwordParameterDefinition, stringParameterDefinition)); + final FreeStyleBuild build1 = j.buildAndAssertSuccess(freeStyleProject); + final FreeStyleBuild build2 = j.waitForCompletion(Objects.requireNonNull(freeStyleProject.scheduleBuild2( + 0, + new ParametersAction( + passwordParameterDefinition.createValue("p4ssw0rd"), + stringParameterDefinition.createValue("value123")))) + .waitForStart()); + { + final HistoryPageFilter historyPageFilter = new HistoryPageFilter<>(30); + historyPageFilter.setSearchString("value"); + historyPageFilter.add(freeStyleProject.getBuilds()); + assertThat(historyPageFilter.runs.stream().map(HistoryPageEntry::getEntry).collect(Collectors.toList()), contains(build2, build1)); + assertThat(historyPageFilter.queueItems, empty()); + } + { + final HistoryPageFilter historyPageFilter = new HistoryPageFilter<>(30); + historyPageFilter.setSearchString("t0p"); + historyPageFilter.add(freeStyleProject.getBuilds()); + assertThat(historyPageFilter.runs, empty()); + assertThat(historyPageFilter.queueItems, empty()); + } + { + final HistoryPageFilter historyPageFilter = new HistoryPageFilter<>(30); + historyPageFilter.setSearchString("value123"); + historyPageFilter.add(freeStyleProject.getBuilds()); + assertThat(historyPageFilter.runs.stream().map(HistoryPageEntry::getEntry).collect(Collectors.toList()), contains(build2)); + assertThat(historyPageFilter.queueItems, empty()); + } + { + final HistoryPageFilter historyPageFilter = new HistoryPageFilter<>(30); + historyPageFilter.setSearchString("p4ss"); + historyPageFilter.add(freeStyleProject.getBuilds()); + assertThat(historyPageFilter.runs, empty()); + assertThat(historyPageFilter.queueItems, empty()); + } + } + + @Test + public void doNotFindSensitiveBuildWrapperVars() throws Exception { + final FreeStyleProject freeStyleProject = j.createFreeStyleProject(); + freeStyleProject.getBuildWrappersList().add(new BuildWrapperWithSomeSensitiveVars(Map.of("key1", "value123", "key2", "value234", "key3", "s3cr3t"), Set.of("key3"))); + final FreeStyleBuild build = j.buildAndAssertSuccess(freeStyleProject); + { + final HistoryPageFilter historyPageFilter = new HistoryPageFilter<>(30); + historyPageFilter.setSearchString("value"); + historyPageFilter.add(freeStyleProject.getBuilds()); + assertThat(historyPageFilter.runs.stream().map(HistoryPageEntry::getEntry).collect(Collectors.toList()), contains(build)); + assertThat(historyPageFilter.queueItems, empty()); + } + { + final HistoryPageFilter historyPageFilter = new HistoryPageFilter<>(30); + historyPageFilter.setSearchString("value123"); + historyPageFilter.add(freeStyleProject.getBuilds()); + assertThat(historyPageFilter.runs.stream().map(HistoryPageEntry::getEntry).collect(Collectors.toList()), contains(build)); + assertThat(historyPageFilter.queueItems, empty()); + } + { + final HistoryPageFilter historyPageFilter = new HistoryPageFilter<>(30); + historyPageFilter.setSearchString("s3cr3t"); + historyPageFilter.add(freeStyleProject.getBuilds()); + assertThat(historyPageFilter.runs, empty()); + assertThat(historyPageFilter.queueItems, empty()); + } + } + + private static class BuildWrapperWithSomeSensitiveVars extends BuildWrapper { + private final Map variables; + private final Set sensitiveVariables; + + private BuildWrapperWithSomeSensitiveVars(Map variables, Set sensitiveVariables) { + + this.variables = new HashMap<>(variables); + this.sensitiveVariables = new HashSet<>(sensitiveVariables); + } + + @Override + public void makeBuildVariables(AbstractBuild build, Map variables) { + variables.putAll(this.variables); + } + + @Override + public void makeSensitiveBuildVariables(AbstractBuild build, Set sensitiveVariables) { + sensitiveVariables.addAll(this.sensitiveVariables); + } + } +}