Skip to content

Commit

Permalink
[SECURITY-3261]
Browse files Browse the repository at this point in the history
  • Loading branch information
@daniel-beck @jenkinsci-cert-ci
daniel-beck authored and jenkinsci-cert-ci committed Sep 6, 2023
1 parent 69b23d0 commit 59dc058
Show file tree
Hide file tree
Showing 3 changed files with 133 additions and 2 deletions.
6 changes: 4 additions & 2 deletions core/src/main/java/jenkins/widgets/HistoryPageFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -371,8 +372,9 @@ private boolean fitsSearchString(Object data) {

private boolean fitsSearchBuildVariables(AbstractBuild<?, ?> runAsBuild) {
Map<String, String> buildVariables = runAsBuild.getBuildVariables();
for (String paramsValues : buildVariables.values()) {
if (fitsSearchString(paramsValues)) {
Set<String> sensitiveBuildVariables = runAsBuild.getSensitiveBuildVariables();
for (Map.Entry<String, String> param : buildVariables.entrySet()) {
if (!sensitiveBuildVariables.contains(param.getKey()) && fitsSearchString(param.getValue())) {
return true;
}
}
Expand Down
7 changes: 7 additions & 0 deletions core/src/test/java/jenkins/widgets/HistoryPageFilterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -485,6 +486,7 @@ private static class MockBuild extends Build<FreeStyleProject, FreeStyleBuild> {
private final int buildNumber;

private Map<String, String> buildVariables = Collections.emptyMap();
private Set<String> sensitiveBuildVariables = Collections.emptySet();

private MockBuild(int buildNumber) {
super(Mockito.mock(FreeStyleProject.class), Mockito.mock(Calendar.class));
Expand All @@ -501,6 +503,11 @@ public Map<String, String> getBuildVariables() {
return buildVariables;
}

@Override
public Set<String> getSensitiveBuildVariables() {
return sensitiveBuildVariables; // TODO This is never actually set (bad Mock), actual test in test harness
}

MockBuild withBuildVariables(Map<String, String> buildVariables) {
this.buildVariables = buildVariables;
return this;
Expand Down
122 changes: 122 additions & 0 deletions test/src/test/java/jenkins/widgets/HistoryPageFilterTest.java
View file
Original file line number Diff line number Diff line change
@@ -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<FreeStyleBuild> 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<FreeStyleBuild> historyPageFilter = new HistoryPageFilter<>(30);
historyPageFilter.setSearchString("t0p");
historyPageFilter.add(freeStyleProject.getBuilds());
assertThat(historyPageFilter.runs, empty());
assertThat(historyPageFilter.queueItems, empty());
}
{
final HistoryPageFilter<FreeStyleBuild> 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<FreeStyleBuild> 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<FreeStyleBuild> 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<FreeStyleBuild> 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<FreeStyleBuild> 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<String, String> variables;
private final Set<String> sensitiveVariables;

private BuildWrapperWithSomeSensitiveVars(Map<String, String> variables, Set<String> sensitiveVariables) {

this.variables = new HashMap<>(variables);
this.sensitiveVariables = new HashSet<>(sensitiveVariables);
}

@Override
public void makeBuildVariables(AbstractBuild build, Map<String, String> variables) {
variables.putAll(this.variables);
}

@Override
public void makeSensitiveBuildVariables(AbstractBuild build, Set<String> sensitiveVariables) {
sensitiveVariables.addAll(this.sensitiveVariables);
}
}
}

0 comments on commit 59dc058

Please sign in to comment.