@gerritbot probably allows users to make arbitrary changes to Phabricator tasks by submitting crafted Gerrit changesets
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	matmarex
	Jul 14 2016, 3:41 PM

Description

@gerritbot probably allows users to make arbitrary changes to Phabricator tasks by submitting crafted Gerrit changesets. I'm saying "probably" because I did not test it – I don't want to test in production and I don't know of any testing instances… I don't think this is a major issue, since the bot doesn't have access to anything special, but it could still cause disruption, so I'm filing this as a security bug.

In T139808#2462319, I noticed that a '+' in the commit message became a ' ', which probably means it's not percent-encoding the data it submits.

Unfortunately the documentation https://wikitech.wikimedia.org/wiki/Gerrit_Notification_Bot points to https://gerrit-review.googlesource.com/#/admin/projects/plugins/its-phabricator as source code, which is definitely not the code that we run. (It's the code that we ran two years ago or so; the comments it posts are different.) This also makes testing the issue locally somewhat more difficult, heh.

Assuming that it still works like that code, it submits parameters as JSON, which means that to set the title task Tnnnn to 'herp' and the description to 'derp', you would submit a commit with the following commit message:

%22%2C%22title%22%3A%22herp%22%2C%22description%22%3A%22derp

Bug: Tnnnn

Details

	Subject	Repo	Branch	Lines +/-
	gerrit: Switch its-phabricator configs from vm to closure template	operations/puppet	production	+3 -15

Customize query in gerrit

Related Objects
Search...

Status	Assigned	Task
Resolved	Paladox	T140366 @gerritbot probably allows users to make arbitrary changes to Phabricator tasks by submitting crafted Gerrit changesets
Resolved	• demon	T156120 Update gerrit to 2.14.6
Open	None	T184086 Add prometheus exporter to Gerrit
Resolved	• demon	T184116 Setup gitiles for use with gerrit as a repo viewer

Event Timeline

matmarex created this task.Jul 14 2016, 3:41 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 14 2016, 3:41 PM

matmarex added a project: GerritBot.Jul 14 2016, 3:41 PM

matmarex removed a subscriber: gerritbot.

greg subscribed.Jul 14 2016, 4:46 PM

Bawolff triaged this task as Medium priority.Jul 19 2016, 8:57 PM

• dpatrick claimed this task.Jul 19 2016, 8:57 PM

greg added a project: Phabricator.Jul 20 2016, 9:49 PM

• mmodell subscribed.Jul 20 2016, 9:50 PM

It would be nice if we had a way to limit specific bot accounts to specific api calls. This would actually be pretty easy to implement in conduit I think.

The changes here would need to happen in its-phabricator (the gerrit plugin, which is java) and I have failed to make any sense of it in the past, gerrit source code is very strange and mysterious to me.

Where is the current source code for gerritbot?

That is indeed the code we run, just older from back in January/February or so of last year. The comments it posts are based on what's in Puppet.

The version for 2.12.2 that's incoming is based on HEAD right now, which is bdbce319842fdd0f2bc800aeb36cd8c03cef5666. This should be branched for stable-2.12, I'll look into that.

The error doesn't appear to be in its-phabricator though, rather its-base which services all of these "talk back to an issue tracker" plugins. We'll be using f2bc13670ef8888e9a1d06d1533b1e889db41a28 which is the current HEAD of stable-2.12 for that repo. If I'm reading it right, it's the $subject parameter (and possibly others) that are given to the *.vm files that need to be properly escaped. This plugin also has a maintainer who we'll want to involve. He's not on our Phabricator but I can put him in touch.

Considering how painful it was (and is) building things for 2.8, I'd like to hold off until after the 2.12.2 upgrade to fix this on our end. I doubt we'll have a fix in the next few days anyway so this shouldn't be much of a blocker.

• demon added a subscriber: QChris.Jul 21 2016, 12:09 AM

Just to be sure, please verify on some test instance that this is really a problem before we make noise upstream.

Hmm… probably not? In Phabricator's syntax, you can suppress all formatting by wrapping a line of text in %%% … %%%. It wouldn't help with lack of URL encoding.

matmarex merged a task: T142925: gerritbot doesn't deal with + in commit message properly.Feb 17 2017, 12:53 PM

matmarex added subscribers: MarkAHershberger, Nemo_bis, Paladox.

Nemo_bis added a subscriber: • Nikerabbit.Feb 17 2017, 2:02 PM

Paladox moved this task from To Triage to Misc on the Phabricator board.Feb 17 2017, 4:59 PM

@matmarex we now run the code hosted on https://gerrit-review.googlesource.com/#/admin/projects/plugins/its-phabricator

Also i have gerrit-test3 and gerrit-test that we can test any changes if you want.

I can reproduce this here https://phab-01.wmflabs.org/T1#2963

though i see no indication of an error in logs.

So i am not sure where to look in the code in its-phabricator to fix this.

This problem could actually be in its-base.

Three reports of this. So i am setting it to high.

Filled upstream as a security task https://bugs.chromium.org/p/gerrit/issues/detail?id=6411

In T140366#3313589, @Paladox wrote:

Three reports of this. So i am setting it to high.

@Paladox: No, three reports over the last years do not suddenly make this task a more urgent problem in June 2017.
Please do not change priority field values without previous consent of the task assignee.

• demon raised the priority of this task from Medium to High.Jun 5 2017, 3:53 PM

Dzahn subscribed.Jun 5 2017, 7:31 PM

This all leads me to its-base. I believe there may be improvements if soy (closure template) was used instead of vm since that has not been updated since 2010. I also do not know how to get vm to read the file as UTF-8, tried everything.

Velocity have released 2.0 after 7 years of no releases, utf-8 is now the default encoding. I wonder will this fix our problem. Not sure if upstream will update it though since they are switching to soy and removing velocity.

I wonder if this https://issues.apache.org/jira/browse/VELOCITY-785 is the same issue here.

Soy seems to support percent encoding by encoding the template as html if chosen to do that. It supports text or html

See response here https://groups.google.com/forum/#!topic/closure-templates-discuss/lXOqqmkO7fE

I am currently converting its-base to soy (closure template)

https://gerrit-review.googlesource.com/#/c/plugins/its-base/+/108215/

Fixed in https://gerrit.wikimedia.org/r/#/c/376406/ requires backend change https://gerrit-review.googlesource.com/#/c/plugins/its-base/+/108215/ .

tested locally and + now shows :).

Paladox claimed this task.Sep 6 2017, 9:05 PM

Paladox added a subscriber: • dpatrick.

Heres the patch for fix when we are ready to deploy it to gerrit.wikimedia.org

patch5 KBDownload

Paladox added a subtask: T156120: Update gerrit to 2.14.6.Sep 6 2017, 9:57 PM

attaching file above also as inline text for readability:

commit 0bf148dba139a1fc8ed07cd1a8376e9a0fc89889
Author: Paladox <thomasmulhall410@yahoo.com>
Date:   Wed Sep 6 22:31:46 2017 +0100

    Gerrit: Convert its base templates to soy (closure template)
    
    This fixes a security issue described in T140366
    
    This change can be merged once we upgrade to 2.14.
    
    and deploy change
    https://gerrit-review.googlesource.com/#/c/plugins/its-base/+/108215/
    
    Bug: T140366
    Change-Id: I2d34a1fefcbef0b730368e5457da7191cef39a92

diff --git a/modules/gerrit/files/etc/its/actions.config b/modules/gerrit/files/etc/its/actions.config
index c9ad7b1f24..006d3247ac 100644
--- a/modules/gerrit/files/etc/its/actions.config
+++ b/modules/gerrit/files/etc/its/actions.config
@@ -10,18 +10,18 @@
 	status = !,DRAFT
 	is-draft = !,true
 	association = subject,footer-Bug,footer-bug
-	action = add-velocity-comment PatchSetMerged
+	action = add-soy-comment PatchSetMerged
 
 [rule "patchSetCreated"]
 	event-type = patchset-created
 	status = !,DRAFT
 	is-draft = !,true
 	association = added@subject,added@footer-Bug,added@footer-bug
-	action = add-velocity-comment PatchSetCreated
+	action = add-soy-comment PatchSetCreated
 	action = add-project Patch-For-Review
 
 [rule "changeDraftPublished"]
 	event-type = draft-published
 	association = added@subject,added@footer-Bug,added@footer-bug
-	action = add-velocity-comment DraftPublished
+	action = add-soy-comment DraftPublished
 	action = add-project Patch-For-Review
diff --git a/modules/gerrit/files/etc/its/templates/DraftPublished.soy b/modules/gerrit/files/etc/its/templates/DraftPublished.soy
new file mode 100644
index 0000000000..7b6c60b0d1
--- /dev/null
+++ b/modules/gerrit/files/etc/its/templates/DraftPublished.soy
@@ -0,0 +1,34 @@
+/**
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+{namespace etc.its.templates}
+
+/**
+ * The .DraftPublished template
+ * @param branch
+ * @param project
+ * @param subject
+ * @param changeNumber
+ * @param authorName
+ * @param formatChangeUrl
+ *
+ */
+{template .DraftPublished autoescape="strict" kind="text"}
+  Change {$changeNumber} had a related patch set (by {$authorName}) published:{\n}
+  [{$project}@{$branch}] {$subject|escapeUri}{\n}{\n}
+
+  {$formatChangeUrl}
+{/template}
diff --git a/modules/gerrit/files/etc/its/templates/PatchSetCreated.soy b/modules/gerrit/files/etc/its/templates/PatchSetCreated.soy
new file mode 100644
index 0000000000..4170244b76
--- /dev/null
+++ b/modules/gerrit/files/etc/its/templates/PatchSetCreated.soy
@@ -0,0 +1,35 @@
+/**
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+{namespace etc.its.templates}
+
+/**
+ * The .PatchSetCreated template
+ * @param branch
+ * @param project
+ * @param subject
+ * @param changeNumber
+ * @param uploaderName
+ * @param authorName
+ * @param formatChangeUrl
+ *
+ */
+{template .PatchSetCreated autoescape="strict" kind="text"}
+  Change {$changeNumber} had a related patch set uploaded (by {$uploaderName}; owner: {$authorName}):{\n}
+  [{$project}@{$branch}] {$subject|escapeUri}{\n}{\n}
+
+  ${formatChangeUrl}
+{/template}
diff --git a/modules/gerrit/files/etc/its/templates/PatchSetMerged.soy b/modules/gerrit/files/etc/its/templates/PatchSetMerged.soy
new file mode 100644
index 0000000000..0dc04dd6f8
--- /dev/null
+++ b/modules/gerrit/files/etc/its/templates/PatchSetMerged.soy
@@ -0,0 +1,34 @@
+/**
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+{namespace etc.its.templates}
+
+/**
+ * The .PatchSetMerged template
+ * @param branch
+ * @param project
+ * @param subject
+ * @param changeNumber
+ * @param submitterName
+ * @param formatChangeUrl
+ *
+ */
+{template .PatchSetMerged autoescape="strict" kind="text"}
+  Change {$changeNumber} merged by {$submitterName}:
+  [{$project}@{$branch}] {$subject|escapeUri}
+
+  ${formatChangeUrl}
+{/template}

thanks :)

we may need to back port the change @demon as i carn't figure out how to fix the tests. It's due to how i am adding a new variable manly for soy as soy has to have a plugin created to add functions but i have no idea on how to do that as there is very little docs on it.

I've managed to fix the tests finally :) :) :).

Just waiting on upstream to merge. Then i will have to backport it to 2.14 since 2.15 drops drafts support.

DraftPublished template is deprecated and will need to be removed when we upgrade to 2.15.

Paladox mentioned this in T158008: Gerrit: Convert Velocity templates to Closure Templates .Oct 4 2017, 8:54 PM

I will try and find someone to merge my changes. I have a lot of changes pending upstream for improvements to the plugins :)

Velocity is no more supported except in 2.14 and 2.15.

Paladox mentioned this in T158243: Update zuul to latest pre 3.0 commit.Oct 24 2017, 2:16 PM

My change has been merged now. I've back ported it to 2.14 here https://gerrit-review.googlesource.com/#/c/plugins/its-base/+/136330/

patch.patch10 KBDownload

attaching file above also as inline text for readability:

commit e38884dacfaeecf77de5d7e81a97067726fe7cc0
Author: Paladox <thomasmulhall410@yahoo.com>
Date:   Wed Oct 25 12:17:43 2017 +0100

    Gerrit: Convert its base templates to soy (closure template)
    
    This fixes a security issue described in T140366
    
    This change can be merged once we upgrade to 2.14.
    
    and deploy change
    https://gerrit-review.googlesource.com/#/c/plugins/its-base/+/108215/
    
    Bug: T140366
    Change-Id: I48eb3f1b60a0cfb4b6f9b611059c495710f8f173

diff --git a/modules/gerrit/files/etc/its/actions.config b/modules/gerrit/files/etc/its/actions.config
index c9ad7b1f24..006d3247ac 100644
--- a/modules/gerrit/files/etc/its/actions.config
+++ b/modules/gerrit/files/etc/its/actions.config
@@ -10,18 +10,18 @@
 	status = !,DRAFT
 	is-draft = !,true
 	association = subject,footer-Bug,footer-bug
-	action = add-velocity-comment PatchSetMerged
+	action = add-soy-comment PatchSetMerged
 
 [rule "patchSetCreated"]
 	event-type = patchset-created
 	status = !,DRAFT
 	is-draft = !,true
 	association = added@subject,added@footer-Bug,added@footer-bug
-	action = add-velocity-comment PatchSetCreated
+	action = add-soy-comment PatchSetCreated
 	action = add-project Patch-For-Review
 
 [rule "changeDraftPublished"]
 	event-type = draft-published
 	association = added@subject,added@footer-Bug,added@footer-bug
-	action = add-velocity-comment DraftPublished
+	action = add-soy-comment DraftPublished
 	action = add-project Patch-For-Review
diff --git a/modules/gerrit/files/etc/its/templates/DraftPublished.soy b/modules/gerrit/files/etc/its/templates/DraftPublished.soy
new file mode 100644
index 0000000000..03f898dfc1
--- /dev/null
+++ b/modules/gerrit/files/etc/its/templates/DraftPublished.soy
@@ -0,0 +1,38 @@
+/**
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+{namespace etc.its.templates}
+
+/**
+ * The .DraftPublished template will determine the contents the published comment
+ * line for all related published comments
+ * @param branch
+ * @param project
+ * @param subject
+ * @param changeNumber
+ * @param authorName
+ * @param authorUsername
+ * @param formatChangeUrl
+ *
+ */
+{template .DraftPublished autoescape="strict" kind="text"}
+
+  Change {$changeNumber} had a related patch set (by {$authorName ? $authorName : $authorUsername}) published:{\n}
+  [{$project}@{$branch}] {$subject|escapeUri}{\n}{\n}
+
+  {$formatChangeUrl}
+{/template}
+
diff --git a/modules/gerrit/files/etc/its/templates/PatchSetCreated.soy b/modules/gerrit/files/etc/its/templates/PatchSetCreated.soy
new file mode 100644
index 0000000000..0c22c9ae9f
--- /dev/null
+++ b/modules/gerrit/files/etc/its/templates/PatchSetCreated.soy
@@ -0,0 +1,40 @@
+/**
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+{namespace etc.its.templates}
+
+/**
+ * The .PatchSetCreated template will determine the contents of the email subject
+ * line for ALL emails related to changes.
+ * @param branch
+ * @param project
+ * @param subject
+ * @param changeNumber
+ * @param authorName
+ * @param authorUsername
+ * @param uploaderName
+ * @param uploaderUsername
+ * @param formatChangeUrl
+ *
+ */
+{template .PatchSetCreated autoescape="strict" kind="text"}
+
+  Change {$changeNumber} had a related patch set uploaded (by {$uploaderName ? $uploaderName : $uploaderUsername}; owner: {$authorName ? $authorName : $authorUsername}):{\n}
+  [{$project}@{$branch}] {$subject|escapeUri}{\n}{\n}
+
+  {$formatChangeUrl}
+{/template}
+
diff --git a/modules/gerrit/files/etc/its/templates/PatchSetMerged.soy b/modules/gerrit/files/etc/its/templates/PatchSetMerged.soy
new file mode 100644
index 0000000000..5747c9bc01
--- /dev/null
+++ b/modules/gerrit/files/etc/its/templates/PatchSetMerged.soy
@@ -0,0 +1,37 @@
+/**
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+{namespace etc.its.templates}
+
+/**
+ * The .PatchSetMerged template will determine the contents of the email subject
+ * line for ALL emails related to changes.
+ * @param changeNumber
+ * @param submitterName
+ * @param submitterUsername
+ * @param project
+ * @param branch
+ * @param subject
+ * @param formatChangeUrl
+ *
+ */
+{template .PatchSetMerged autoescape="strict" kind="text"}
+  Change {$changeNumber} merged by {$submitterName ? $submitterName : $submitterUsername}:{\n}
+  [{$project}@{$branch}] {$subject|escapeUri}{\n}{\n}
+
+  {$formatChangeUrl}
+{/template}
+
diff --git a/modules/gerrit/files/etc/its/templates/patch.patch b/modules/gerrit/files/etc/its/templates/patch.patch
new file mode 100644
index 0000000000..56fe6e978f
--- /dev/null
+++ b/modules/gerrit/files/etc/its/templates/patch.patch
@@ -0,0 +1,133 @@
+diff --git a/modules/gerrit/files/etc/its/templates/DraftPublished.soy b/modules/gerrit/files/etc/its/templates/DraftPublished.soy
+new file mode 100644
+index 0000000000..03f898dfc1
+--- /dev/null
++++ b/modules/gerrit/files/etc/its/templates/DraftPublished.soy
+@@ -0,0 +1,38 @@
++/**
++ * Copyright (C) 2017 The Android Open Source Project
++ *
++ * Licensed under the Apache License, Version 2.0 (the "License");
++ * you may not use this file except in compliance with the License.
++ * You may obtain a copy of the License at
++ *
++ * http://www.apache.org/licenses/LICENSE-2.0
++ *
++ * Unless required by applicable law or agreed to in writing, software
++ * distributed under the License is distributed on an "AS IS" BASIS,
++ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
++ * See the License for the specific language governing permissions and
++ * limitations under the License.
++ */
++
++{namespace etc.its.templates}
++
++/**
++ * The .DraftPublished template will determine the contents the published comment
++ * line for all related published comments
++ * @param branch
++ * @param project
++ * @param subject
++ * @param changeNumber
++ * @param authorName
++ * @param authorUsername
++ * @param formatChangeUrl
++ *
++ */
++{template .DraftPublished autoescape="strict" kind="text"}
++
++  Change {$changeNumber} had a related patch set (by {$authorName ? $authorName : $authorUsername}) published:{\n}
++  [{$project}@{$branch}] {$subject|escapeUri}{\n}{\n}
++
++  {$formatChangeUrl}
++{/template}
++
+diff --git a/modules/gerrit/files/etc/its/templates/PatchSetCreated.soy b/modules/gerrit/files/etc/its/templates/PatchSetCreated.soy
+new file mode 100644
+index 0000000000..0c22c9ae9f
+--- /dev/null
++++ b/modules/gerrit/files/etc/its/templates/PatchSetCreated.soy
+@@ -0,0 +1,40 @@
++/**
++ * Copyright (C) 2017 The Android Open Source Project
++ *
++ * Licensed under the Apache License, Version 2.0 (the "License");
++ * you may not use this file except in compliance with the License.
++ * You may obtain a copy of the License at
++ *
++ * http://www.apache.org/licenses/LICENSE-2.0
++ *
++ * Unless required by applicable law or agreed to in writing, software
++ * distributed under the License is distributed on an "AS IS" BASIS,
++ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
++ * See the License for the specific language governing permissions and
++ * limitations under the License.
++ */
++
++{namespace etc.its.templates}
++
++/**
++ * The .PatchSetCreated template will determine the contents of the email subject
++ * line for ALL emails related to changes.
++ * @param branch
++ * @param project
++ * @param subject
++ * @param changeNumber
++ * @param authorName
++ * @param authorUsername
++ * @param uploaderName
++ * @param uploaderUsername
++ * @param formatChangeUrl
++ *
++ */
++{template .PatchSetCreated autoescape="strict" kind="text"}
++
++  Change {$changeNumber} had a related patch set uploaded (by {$uploaderName ? $uploaderName : $uploaderUsername}; owner: {$authorName ? $authorName : $authorUsername}):{\n}
++  [{$project}@{$branch}] {$subject|escapeUri}{\n}{\n}
++
++  {$formatChangeUrl}
++{/template}
++
+diff --git a/modules/gerrit/files/etc/its/templates/PatchSetMerged.soy b/modules/gerrit/files/etc/its/templates/PatchSetMerged.soy
+new file mode 100644
+index 0000000000..5747c9bc01
+--- /dev/null
++++ b/modules/gerrit/files/etc/its/templates/PatchSetMerged.soy
+@@ -0,0 +1,37 @@
++/**
++ * Copyright (C) 2017 The Android Open Source Project
++ *
++ * Licensed under the Apache License, Version 2.0 (the "License");
++ * you may not use this file except in compliance with the License.
++ * You may obtain a copy of the License at
++ *
++ * http://www.apache.org/licenses/LICENSE-2.0
++ *
++ * Unless required by applicable law or agreed to in writing, software
++ * distributed under the License is distributed on an "AS IS" BASIS,
++ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
++ * See the License for the specific language governing permissions and
++ * limitations under the License.
++ */
++
++{namespace etc.its.templates}
++
++/**
++ * The .PatchSetMerged template will determine the contents of the email subject
++ * line for ALL emails related to changes.
++ * @param changeNumber
++ * @param submitterName
++ * @param submitterUsername
++ * @param project
++ * @param branch
++ * @param subject
++ * @param formatChangeUrl
++ *
++ */
++{template .PatchSetMerged autoescape="strict" kind="text"}
++  Change {$changeNumber} merged by {$submitterName ? $submitterName : $submitterUsername}:{\n}
++  [{$project}@{$branch}] {$subject|escapeUri}{\n}{\n}
++
++  {$formatChangeUrl}
++{/template}
++

we can use ternary expressions now :).

which i have done as i found that if someone dosen't have a name but username it will cause the template to not work.

so i did <name> ? <name> : <username> (username as fallback for when name is undefined) It's better then it breaking and not working :).

We will be updating to gerrit 2.14 on friday 2nd of feb. So this will be deployed then. Changes will be uploaded at the time we upgrade.

https://gerrit.wikimedia.org/r/#/c/407753/

This is now resolved. Please give this another try.

Paladox closed this task as Resolved.Feb 3 2018, 12:53 AM

Paladox closed subtask T156120: Update gerrit to 2.14.6 as Resolved.Feb 3 2018, 1:04 AM

• demon changed the visibility from "Custom Policy" to "Public (No Login Required)".Feb 3 2018, 1:05 AM

Paladox mentioned this in T188033: its-phabricator (gerritbot) sometimes not writing to tasks.Feb 22 2018, 10:00 PM

• chasemp added a project: Security.Feb 10 2020, 10:55 PM

• chasemp removed a project: acl*security.Feb 20 2020, 8:15 PM

	F10427069: patch.patch
	Oct 25 2017, 11:20 AM

@gerritbot probably allows users to make arbitrary changes to Phabricator tasks by submitting crafted Gerrit changesetsClosed, ResolvedPublicActions

Description

Details

Related ObjectsSearch...

Event Timeline

@gerritbot probably allows users to make arbitrary changes to Phabricator tasks by submitting crafted Gerrit changesets
Closed, ResolvedPublic
Actions

Related Objects
Search...