Skip to content

Commit 31fd5b9

Browse files
daniel-beckjtnord
daniel-beck
and
jtnord
committed
Compatibility with SECURITY-2880 fix
Co-authored-by: James Nord <jtnord@users.noreply.github.com>
1 parent 9b5cc3a commit 31fd5b9

File tree

5 files changed

+66
-15
lines changed

5 files changed

+66
-15
lines changed

‎pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/RuntimeASTTransformer.groovy‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
package org.jenkinsci.plugins.pipeline.modeldefinition.parser
2626

2727
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings
28+
import hudson.Util
2829
import hudson.model.JobProperty
2930
import hudson.model.ParameterDefinition
3031
import hudson.model.Run
@@ -742,9 +743,15 @@ public class RuntimeASTTransformer {
742743
if (!original.parameters.isEmpty()) {
743744
paramsExpr = wrapper.asWrappedScriptContextVariable(transformListOfDescribables(original.parameters, ParameterDefinition.class))
744745
}
746+
747+
String id = stageName
748+
if (stageName != null && !SystemProperties.getBoolean(RuntimeASTTransformer.class.name + '.retainOriginalStageNameForInputId')) {
749+
id = Util.getDigestOf(stageName)
750+
}
751+
745752
return wrapper.asExternalMethodCall(ctorX(ClassHelper.make(StageInput.class),
746753
args(valueOrNull(original.message),
747-
valueOrNull(original.id, stageName),
754+
valueOrNull(original.id, id),
748755
valueOrNull(original.ok),
749756
valueOrNull(original.submitter),
750757
valueOrNull(original.submitterParameter),

‎pipeline-model-definition/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/generator/InputDirective.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,44 @@ public FormValidation doCheckMessage(@QueryParameter String value) {
134134
}
135135
}
136136

137+
public FormValidation doCheckId(@QueryParameter String id) {
138+
// TODO post SECURITY-2880 update the pipeline-input-step dependency and call the InputStep descriptor check
139+
140+
// https://www.rfc-editor.org/rfc/rfc3986.txt
141+
// URLs may only contain ascii
142+
// and only some parts are allowed
143+
// segment = *pchar
144+
// segment-nz = 1*pchar
145+
// segment-nz-nc = 1*( unreserved / pct-encoded / sub-delims / "@" )
146+
// ; non-zero-length segment without any colon ":"
147+
// pchar = unreserved / pct-encoded / sub-delims / ":" / "@"
148+
// unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~"
149+
// sub-delims = "!" / "$" / "&" / "'" / "(" / ")"
150+
// / "*" / "+" / "," / ";" / "="
151+
152+
// but we are not allowing pct-encoded here.
153+
// additionally "." and ".." should be rejected.
154+
// and as we are using html / javascript in places we disallow "'"
155+
// and to prevent escaping hell disallow "&"
156+
157+
// as well as anything unsafe we disallow . and .. (but we can have a dot inside the string so foo.bar is ok)
158+
// also Jenkins dissallows ; in the request parameter so don't allow that either.
159+
if (id == null || id.isEmpty()) {
160+
// the id will be provided by a hash of the message
161+
return FormValidation.ok();
162+
}
163+
if (id.equals(".")) {
164+
return FormValidation.error("The ID is required to be URL safe and is limited to the characters a-z A-Z, the digits 0-9 and additionally the characters ':' '@' '=' '+' '$' ',' '-' '_' '.' '!' '~' '*' '(' ')'.");
165+
}
166+
if (id.equals("..")) {
167+
return FormValidation.error("The ID is required to be URL safe and is limited to the characters a-z A-Z, the digits 0-9 and additionally the characters ':' '@' '=' '+' '$' ',' '-' '_' '.' '!' '~' '*' '(' ')'.");
168+
}
169+
if (!id.matches("^[a-zA-Z0-9[-]._~!$()*+,:@=]+$")) { // escape the - inside another [] so it does not become a range of , - _
170+
return FormValidation.error("The ID is required to be URL safe and is limited to the characters a-z A-Z, the digits 0-9 and additionally the characters ':' '@' '=' '+' '$' ',' '-' '_' '.' '!' '~' '*' '(' ')'.");
171+
}
172+
return FormValidation.ok();
173+
}
174+
137175
@Override
138176
@NonNull
139177
public String toGroovy(@NonNull InputDirective directive) {

‎pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/MatrixTest.java‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,12 @@
2424
package org.jenkinsci.plugins.pipeline.modeldefinition;
2525

2626
import com.gargoylesoftware.htmlunit.html.HtmlPage;
27+
import hudson.Util;
2728
import hudson.model.Result;
2829
import hudson.model.Slave;
2930
import hudson.model.queue.QueueTaskFuture;
3031
import hudson.slaves.EnvironmentVariablesNodeProperty;
32+
import org.apache.commons.lang.StringUtils;
3133
import org.jenkinsci.plugins.pipeline.StageStatus;
3234
import org.jenkinsci.plugins.pipeline.modeldefinition.parser.RuntimeASTTransformer;
3335
import org.jenkinsci.plugins.workflow.actions.TagsAction;
@@ -774,7 +776,7 @@ public void matrixInput() throws Exception {
774776
JenkinsRule.WebClient wc = j.createWebClient();
775777
HtmlPage page;
776778

777-
InputStepExecution is1 = a.getExecution("Matrix - AXIS_VALUE = 'A'");
779+
InputStepExecution is1 = a.getExecution(StringUtils.capitalize(Util.getDigestOf("Matrix - AXIS_VALUE = 'A'")));
778780
assertEquals("Continue?", is1.getInput().getMessage());
779781
assertEquals(0, is1.getInput().getParameters().size());
780782
assertNull(is1.getInput().getSubmitter());
@@ -784,7 +786,7 @@ public void matrixInput() throws Exception {
784786

785787
assertEquals(1, a.getExecutions().size());
786788

787-
is1 = a.getExecution("Matrix - AXIS_VALUE = 'B'");
789+
is1 = a.getExecution(StringUtils.capitalize(Util.getDigestOf("Matrix - AXIS_VALUE = 'B'")));
788790
assertEquals("Continue?", is1.getInput().getMessage());
789791
assertEquals(0, is1.getInput().getParameters().size());
790792
assertNull(is1.getInput().getSubmitter());
@@ -824,7 +826,7 @@ public void matrixInputChildStage() throws Exception {
824826
JenkinsRule.WebClient wc = j.createWebClient();
825827
HtmlPage page;
826828

827-
InputStepExecution is1 = a.getExecution("Cell");
829+
InputStepExecution is1 = a.getExecution(StringUtils.capitalize(Util.getDigestOf("Cell")));
828830
assertEquals("Continue?", is1.getInput().getMessage());
829831
assertEquals(0, is1.getInput().getParameters().size());
830832
assertNull(is1.getInput().getSubmitter());
@@ -834,7 +836,7 @@ public void matrixInputChildStage() throws Exception {
834836

835837
assertEquals(1, a.getExecutions().size());
836838

837-
is1 = a.getExecution("Cell");
839+
is1 = a.getExecution(StringUtils.capitalize(Util.getDigestOf("Cell")));
838840
assertEquals("Continue?", is1.getInput().getMessage());
839841
assertEquals(0, is1.getInput().getParameters().size());
840842
assertNull(is1.getInput().getSubmitter());
@@ -874,7 +876,7 @@ public void matrixInputWhen() throws Exception {
874876
JenkinsRule.WebClient wc = j.createWebClient();
875877
HtmlPage page;
876878

877-
InputStepExecution is1 = a.getExecution("Matrix - AXIS_VALUE = 'A'");
879+
InputStepExecution is1 = a.getExecution(StringUtils.capitalize(Util.getDigestOf("Matrix - AXIS_VALUE = 'A'")));
878880
assertEquals("Continue?", is1.getInput().getMessage());
879881
assertEquals(0, is1.getInput().getParameters().size());
880882
assertNull(is1.getInput().getSubmitter());
@@ -884,7 +886,7 @@ public void matrixInputWhen() throws Exception {
884886

885887
assertEquals(1, a.getExecutions().size());
886888

887-
is1 = a.getExecution("Matrix - AXIS_VALUE = 'B'");
889+
is1 = a.getExecution(StringUtils.capitalize(Util.getDigestOf("Matrix - AXIS_VALUE = 'B'")));
888890
assertEquals("Continue?", is1.getInput().getMessage());
889891
assertEquals(0, is1.getInput().getParameters().size());
890892
assertNull(is1.getInput().getSubmitter());
@@ -925,7 +927,7 @@ public void matrixInputWhenChildStage() throws Exception {
925927
JenkinsRule.WebClient wc = j.createWebClient();
926928
HtmlPage page;
927929

928-
InputStepExecution is1 = a.getExecution("Cell");
930+
InputStepExecution is1 = a.getExecution(StringUtils.capitalize(Util.getDigestOf("Cell")));
929931
assertEquals("Continue?", is1.getInput().getMessage());
930932
assertEquals(0, is1.getInput().getParameters().size());
931933
assertNull(is1.getInput().getSubmitter());
@@ -935,7 +937,7 @@ public void matrixInputWhenChildStage() throws Exception {
935937

936938
assertEquals(1, a.getExecutions().size());
937939

938-
is1 = a.getExecution("Cell");
940+
is1 = a.getExecution(StringUtils.capitalize(Util.getDigestOf("Cell")));
939941
assertEquals("Continue?", is1.getInput().getMessage());
940942
assertEquals(0, is1.getInput().getParameters().size());
941943
assertNull(is1.getInput().getSubmitter());
@@ -976,7 +978,7 @@ public void matrixInputWhenBeforeInput() throws Exception {
976978
JenkinsRule.WebClient wc = j.createWebClient();
977979
HtmlPage page;
978980

979-
InputStepExecution is1 = a.getExecution("Matrix - AXIS_VALUE = 'A'");
981+
InputStepExecution is1 = a.getExecution(StringUtils.capitalize(Util.getDigestOf("Matrix - AXIS_VALUE = 'A'")));
980982
assertEquals("Continue?", is1.getInput().getMessage());
981983
assertEquals(0, is1.getInput().getParameters().size());
982984
assertNull(is1.getInput().getSubmitter());
@@ -1017,7 +1019,7 @@ public void matrixInputWhenBeforeInputChildStage() throws Exception {
10171019
JenkinsRule.WebClient wc = j.createWebClient();
10181020
HtmlPage page;
10191021

1020-
InputStepExecution is1 = a.getExecution("Cell");
1022+
InputStepExecution is1 = a.getExecution(StringUtils.capitalize(Util.getDigestOf("Cell")));
10211023
assertEquals("Continue?", is1.getInput().getMessage());
10221024
assertEquals(0, is1.getInput().getParameters().size());
10231025
assertNull(is1.getInput().getSubmitter());

‎pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/ParallelTest.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,12 @@
2424
package org.jenkinsci.plugins.pipeline.modeldefinition;
2525

2626
import com.gargoylesoftware.htmlunit.html.HtmlPage;
27+
import hudson.Util;
2728
import hudson.model.Result;
2829
import hudson.model.Slave;
2930
import hudson.model.queue.QueueTaskFuture;
3031
import hudson.slaves.EnvironmentVariablesNodeProperty;
32+
import org.apache.commons.lang.StringUtils;
3133
import org.jenkinsci.plugins.pipeline.StageStatus;
3234
import org.jenkinsci.plugins.workflow.actions.TagsAction;
3335
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
@@ -415,12 +417,12 @@ public void parallelInput() throws Exception {
415417
InputAction a = b.getAction(InputAction.class);
416418
assertEquals(2, a.getExecutions().size());
417419

418-
InputStepExecution is1 = a.getExecution("One");
420+
InputStepExecution is1 = a.getExecution(StringUtils.capitalize(Util.getDigestOf("One")));
419421
assertEquals("Continue One?", is1.getInput().getMessage());
420422
assertEquals(0, is1.getInput().getParameters().size());
421423
assertNull(is1.getInput().getSubmitter());
422424

423-
InputStepExecution is2 = a.getExecution("Two");
425+
InputStepExecution is2 = a.getExecution(StringUtils.capitalize(Util.getDigestOf("Two")));
424426
assertEquals("Continue Two?", is2.getInput().getMessage());
425427
assertEquals(0, is2.getInput().getParameters().size());
426428
assertNull(is2.getInput().getSubmitter());

‎pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/StageInputTest.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@
2525
package org.jenkinsci.plugins.pipeline.modeldefinition;
2626

2727
import com.gargoylesoftware.htmlunit.html.*;
28+
import hudson.Util;
2829
import hudson.model.queue.QueueTaskFuture;
30+
import org.apache.commons.lang.StringUtils;
2931
import org.jenkinsci.plugins.pipeline.modeldefinition.parser.RuntimeASTTransformer;
3032
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
3133
import org.jenkinsci.plugins.workflow.cps.CpsFlowExecution;
@@ -60,7 +62,7 @@ public void simpleInput() throws Exception {
6062
InputAction a = b.getAction(InputAction.class);
6163
assertEquals(1, a.getExecutions().size());
6264

63-
InputStepExecution is = a.getExecution("Foo");
65+
InputStepExecution is = a.getExecution(StringUtils.capitalize(Util.getDigestOf("foo")));
6466
assertEquals("Continue?", is.getInput().getMessage());
6567
assertEquals(0, is.getInput().getParameters().size());
6668
assertNull(is.getInput().getSubmitter());
@@ -96,7 +98,7 @@ public void simpleInputWithOutsideVarAndFunc() throws Exception {
9698
InputAction a = b.getAction(InputAction.class);
9799
assertEquals(1, a.getExecutions().size());
98100

99-
InputStepExecution is = a.getExecution("Foo");
101+
InputStepExecution is = a.getExecution(StringUtils.capitalize(Util.getDigestOf("foo")));
100102
assertEquals("Continue?", is.getInput().getMessage());
101103
assertEquals(0, is.getInput().getParameters().size());
102104
assertNull(is.getInput().getSubmitter());

0 commit comments

Comments
 (0)