Skip to content
Permalink
Browse files Browse the repository at this point in the history
[SECURITY-55]
This patch makes standard post-build action refuse to let you configure a downstream project you cannot currently build.
The one from parameterized-trigger will show an error in the configure screen but still lets you save the configuration; needs an analogous patch to that plugin.
Does not yet protect against POSTing config.xml with the trigger.
(cherry picked from commit 757bc8a)

Conflicts:

	core/src/main/java/hudson/model/Descriptor.java
  • Loading branch information
jglick authored and kohsuke committed Feb 13, 2013
1 parent 0de3e9b commit 36342d7
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 5 deletions.
7 changes: 7 additions & 0 deletions core/src/main/java/hudson/model/AbstractProject.java
Expand Up @@ -1914,6 +1914,13 @@ protected void submit(StaplerRequest req, StaplerResponse rsp) throws IOExceptio
triggers = buildDescribable(req, Trigger.for_(this));
for (Trigger t : triggers)
t.start(this,true);

for (Publisher _t : Descriptor.newInstancesFromHeteroList(req, json, "publisher", Jenkins.getInstance().getExtensionList(BuildTrigger.DescriptorImpl.class))) {
BuildTrigger t = (BuildTrigger) _t;

This comment has been minimized.

Copy link
@kutzi

kutzi Feb 21, 2013

Member

This causes https://issues.jenkins-ci.org/browse/JENKINS-16917
I admit that the extension of the BuildTrigger.DescriptorImpl in DownstreamTrigger is a bit 'unconventional' (see: https://github.com/jenkinsci/downstream-ext-plugin/blob/master/src/main/java/hudson/plugins/downstream_ext/DownstreamTrigger.java#L296)
but if BuildTrigger.DescriptorImp isn't meant to be extendable it should IMHO be made final.

for (AbstractProject downstream : t.getChildProjects(this)) {
downstream.checkPermission(BUILD);
}
}
}

/**
Expand Down
9 changes: 6 additions & 3 deletions core/src/main/java/hudson/model/Descriptor.java
Expand Up @@ -936,7 +936,10 @@ List<T> newInstancesFromHeteroList(StaplerRequest req, Object formData,
for (Object o : JSONArray.fromObject(formData)) {
JSONObject jo = (JSONObject)o;
String kind = jo.getString("kind");
items.add(find(descriptors,kind).newInstance(req,jo));
Descriptor<T> d = find(descriptors, kind);
if (d != null) {
items.add(d.newInstance(req, jo));
}
}
}

Expand All @@ -946,7 +949,7 @@ List<T> newInstancesFromHeteroList(StaplerRequest req, Object formData,
/**
* Finds a descriptor from a collection by its class name.
*/
public static <T extends Descriptor> T find(Collection<? extends T> list, String className) {
public static @CheckForNull <T extends Descriptor> T find(Collection<? extends T> list, String className) {
for (T d : list) {
if(d.getClass().getName().equals(className))
return d;
Expand All @@ -960,7 +963,7 @@ public static <T extends Descriptor> T find(Collection<? extends T> list, String
return null;
}

public static Descriptor find(String className) {
public static @CheckForNull Descriptor find(String className) {
return find(Jenkins.getInstance().getExtensionList(Descriptor.class),className);
}

Expand Down
5 changes: 4 additions & 1 deletion core/src/main/java/hudson/tasks/BuildTrigger.java
Expand Up @@ -319,7 +319,7 @@ public boolean showEvenIfUnstableOption(@CheckForNull Class<? extends AbstractPr
/**
* Form validation method.
*/
public FormValidation doCheck(@AncestorInPath Item project, @QueryParameter String value ) {
public FormValidation doCheck(@AncestorInPath Item project, @QueryParameter String value, @QueryParameter boolean upstream) {
// Require CONFIGURE permission on this project
if(!project.hasPermission(Item.CONFIGURE)) return FormValidation.ok();

Expand All @@ -334,6 +334,9 @@ public FormValidation doCheck(@AncestorInPath Item project, @QueryParameter Stri
AbstractProject.findNearest(projectName,project.getParent()).getRelativeNameFrom(project)));
if(!(item instanceof AbstractProject))
return FormValidation.error(Messages.BuildTrigger_NotBuildable(projectName));
if (!upstream && !item.hasPermission(Item.BUILD)) {
return FormValidation.error(Messages.BuildTrigger_you_have_no_permission_to_build_(projectName));
}
hasProjects = true;
}
}
Expand Down
1 change: 1 addition & 0 deletions core/src/main/resources/hudson/tasks/Messages.properties
Expand Up @@ -47,6 +47,7 @@ BuildTrigger.NoSuchProject=No such project ''{0}''. Did you mean ''{1}''?
BuildTrigger.NoProjectSpecified=No project specified
BuildTrigger.NotBuildable={0} is not buildable
BuildTrigger.Triggering=Triggering a new build of {0}
BuildTrigger.you_have_no_permission_to_build_=You have no permission to build {0}

CommandInterpreter.CommandFailed=command execution failed
CommandInterpreter.UnableToDelete=Unable to delete script file {0}
Expand Down
Expand Up @@ -38,7 +38,7 @@ THE SOFTWARE.
<f:entry title="${%Project names}"
description="${%Multiple projects can be specified like 'abc, def'}">
<f:textbox name="upstreamProjects" value="${h.getProjectListString(up)}"
checkUrl="'descriptorByName/hudson.tasks.BuildTrigger/check?value='+encodeURIComponent(this.value)"
checkUrl="'descriptorByName/hudson.tasks.BuildTrigger/check?upstream=true&amp;value='+encodeURIComponent(this.value)"
autoCompleteField="upstreamProjects"/>
</f:entry>
</f:optionalBlock>
Expand Down

0 comments on commit 36342d7

Please sign in to comment.