[error-report-web,V2] Add local.conf and auto.conf into error details

Submitted by changqing.li@windriver.com on Nov. 12, 2019, 8:32 a.m. | Patch ID: 166875

Details

Message ID 1573547573-352969-1-git-send-email-changqing.li@windriver.com
State New
Headers show

Commit Message

changqing.li@windriver.com Nov. 12, 2019, 8:32 a.m.
From: Changqing Li <changqing.li@windriver.com>

Support to display local.conf and auto.conf on error report web.
Here is commit in oe-core, which add local.conf/auto.conf into error report
https://git.openembedded.org/openembedded-core/commit/?id=7adf9707c04d8ef6bcd8d8bda555687f705e6ee6

This commit is related to YOCTO #13252

Signed-off-by: Changqing Li <changqing.li@windriver.com>
---
 Post/0006_auto_20190917_0419.py | 24 ++++++++++++++++++++++++
 Post/models.py                  |  2 ++
 Post/parser.py                  |  2 ++
 Post/test.py                    |  2 ++
 templates/error-details.html    | 10 ++++++++++
 test-data/test-payload.json     |  4 +++-
 6 files changed, 43 insertions(+), 1 deletion(-)
 create mode 100644 Post/0006_auto_20190917_0419.py

Patch hide | download patch | download mbox

diff --git a/Post/0006_auto_20190917_0419.py b/Post/0006_auto_20190917_0419.py
new file mode 100644
index 0000000..827944e
--- /dev/null
+++ b/Post/0006_auto_20190917_0419.py
@@ -0,0 +1,24 @@ 
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('Post', '0005_build_error_type'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='build',
+            name='AUTO_CONF',
+            field=models.TextField(default=b'', max_length=5242880),
+        ),
+        migrations.AddField(
+            model_name='build',
+            name='LOCAL_CONF',
+            field=models.TextField(default=b'', max_length=5242880),
+        ),
+    ]
diff --git a/Post/models.py b/Post/models.py
index ddf2fc7..e9fd010 100644
--- a/Post/models.py
+++ b/Post/models.py
@@ -43,6 +43,8 @@  class Build(models.Model):
     LINK_BACK = models.TextField(max_length=300, blank=True, null=True)
     ERROR_TYPE = models.CharField(max_length=20, choices=ERROR_TYPE_CHOICES,
                                   default=ErrorType.RECIPE)
+    LOCAL_CONF = models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
+    AUTO_CONF = models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
 
     def save(self, *args, **kwargs):
         if self.ERROR_TYPE not in [e_type[0] for e_type in
diff --git a/Post/parser.py b/Post/parser.py
index 9639308..69d43c1 100644
--- a/Post/parser.py
+++ b/Post/parser.py
@@ -53,6 +53,8 @@  class Parser:
             b.EMAIL = str(jsondata['email'])
             b.LINK_BACK = jsondata.get("link_back", None)
             b.ERROR_TYPE = jsondata.get("error_type", ErrorType.RECIPE)
+            b.LOCAL_CONF = str(jsondata['local_conf'])
+            b.AUTO_CONF = str(jsondata['auto_conf'])
 
             # Extract the branch and commit
             g = re.match(r'(.*): (.*)', jsondata['branch_commit'])
diff --git a/Post/test.py b/Post/test.py
index 6dc7878..4fe236c 100755
--- a/Post/test.py
+++ b/Post/test.py
@@ -46,6 +46,8 @@  def compare_db_obj_with_payload(self, bf_object):
     self.assertEqual(bf_object.BUILD.NAME == str(payload['username']), True)
     self.assertEqual(bf_object.BUILD.EMAIL == str(payload['email']), True)
     self.assertEqual(bf_object.BUILD.LINK_BACK == payload.get("link_back", None), True)
+    self.assertEqual(bf_object.BUILD.LOCAL_CONF == payload.get("local_conf", None), True)
+    self.assertEqual(bf_object.BUILD.AUTO_CONF == payload.get("auto_conf", None), True)
 
     g = re.match(r'(.*): (.*)', payload['branch_commit'])
 
diff --git a/templates/error-details.html b/templates/error-details.html
index c30160d..c8fce05 100644
--- a/templates/error-details.html
+++ b/templates/error-details.html
@@ -81,6 +81,16 @@ 
           {% endwith %}
         </dd>
 
+        {% if detail.BUILD.LOCAL_CONF != "" %}
+        <dt></a>Local Conf:</dt>
+        <dd style="white-space: pre-wrap;">{{ detail.BUILD.LOCAL_CONF | safe }}</dd>
+        {% endif %}
+
+        {% if detail.BUILD.AUTO_CONF != "" %}
+        <dt></a>Auto Conf:</dt>
+        <dd style="white-space: pre-wrap;">{{ detail.BUILD.AUTO_CONF | safe }}</dd>
+        {% endif %}
+
       </dl>
       <div>
         <a class="btn btn-block" target="_blank" href="https://bugzilla.yoctoproject.org/enter_bug.cgi?classification=__all" >Open a bug</a>
diff --git a/test-data/test-payload.json b/test-data/test-payload.json
index 0428a16..fd21053 100644
--- a/test-data/test-payload.json
+++ b/test-data/test-payload.json
@@ -1,4 +1,5 @@ 
 {
+    "auto_conf": "",
     "branch_commit": "(master-test): 736b49449233c936e3099b314a58e91ad17f9774", 
     "build_sys": "x86_64-linux", 
     "component": "base-passwd", 
@@ -11,7 +12,8 @@ 
             "package": "base-passwd-3.5.29-r0", 
             "task": "do_install"
         }
-    ], 
+    ],
+    "local_conf": "",
     "machine": "qemux86", 
     "nativelsb": "Ubuntu-14.04", 
     "target_sys": "i586-poky-linux"

Comments

Paul Eggleton Nov. 13, 2019, 10:36 a.m.
Hi Changqing,

Some comments below.

On Tuesday, 12 November 2019 9:32:53 PM NZDT changqing.li@windriver.com wrote:
> From: Changqing Li <changqing.li@windriver.com>
> 
> Support to display local.conf and auto.conf on error report web.
> Here is commit in oe-core, which add local.conf/auto.conf into error report
> https://git.openembedded.org/openembedded-core/commit/?id=7adf9707c04d8ef6bcd8d8bda555687f705e6ee6
> 
> This commit is related to YOCTO #13252
> 
> Signed-off-by: Changqing Li <changqing.li@windriver.com>
> ---
>  Post/0006_auto_20190917_0419.py | 24 ++++++++++++++++++++++++
>  Post/models.py                  |  2 ++
>  Post/parser.py                  |  2 ++
>  Post/test.py                    |  2 ++
>  templates/error-details.html    | 10 ++++++++++
>  test-data/test-payload.json     |  4 +++-
>  6 files changed, 43 insertions(+), 1 deletion(-)
>  create mode 100644 Post/0006_auto_20190917_0419.py
> 
> diff --git a/Post/0006_auto_20190917_0419.py b/Post/0006_auto_20190917_0419.py
> new file mode 100644
> index 0000000..827944e
> --- /dev/null
> +++ b/Post/0006_auto_20190917_0419.py

Could you please give the migration a proper name (-n option to makemigrations) e.g. local_conf_auto_conf

> --- a/Post/models.py
> +++ b/Post/models.py
> @@ -43,6 +43,8 @@ class Build(models.Model):
>      LINK_BACK = models.TextField(max_length=300, blank=True, null=True)
>      ERROR_TYPE = models.CharField(max_length=20, choices=ERROR_TYPE_CHOICES,
>                                    default=ErrorType.RECIPE)
> +    LOCAL_CONF = models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
> +    AUTO_CONF = models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")

I'm not sure this is practical, for two reasons:

1) Field sizes should not be variable like this; changing the MAX_UPLOAD_SIZE value after the fact would not change the database structure
2) The value could never actually reach MAX_UPLOAD_SIZE because the overhead of the surrounding JSON would block it from being uploaded if it did

However, since this is a TextField we don't actually have to specify a max_length (for a TextField max_length only actually affects the frontend, and we don't expose this field in a form) so it can just be removed.

Another thing, instead of default="" you should use blank=True.


> +        {% if detail.BUILD.LOCAL_CONF != "" %}
> +        <dt></a>Local Conf:</dt>
> +        <dd style="white-space: pre-wrap;">{{ detail.BUILD.LOCAL_CONF | safe }}</dd>
> +        {% endif %}
> +
> +        {% if detail.BUILD.AUTO_CONF != "" %}
> +        <dt></a>Auto Conf:</dt>
> +        <dd style="white-space: pre-wrap;">{{ detail.BUILD.AUTO_CONF | safe }}</dd>
> +        {% endif %}

We cannot use the safe filter here - doing so could open up an XSS vulnerability, since anyone can upload anything to the error-report application and the content could include links or other malicious HTML data. We should allow it to be auto-escaped. Is there a particular issue you were using this to solve?

Cheers
Paul
changqing.li@windriver.com Dec. 11, 2019, 5:45 a.m.
On 11/13/19 6:36 PM, Paul Eggleton wrote:
> Hi Changqing,
>
> Some comments below.
>
> On Tuesday, 12 November 2019 9:32:53 PM NZDT changqing.li@windriver.com wrote:
>> From: Changqing Li <changqing.li@windriver.com>
>>
>> Support to display local.conf and auto.conf on error report web.
>> Here is commit in oe-core, which add local.conf/auto.conf into error report
>> https://git.openembedded.org/openembedded-core/commit/?id=7adf9707c04d8ef6bcd8d8bda555687f705e6ee6
>>
>> This commit is related to YOCTO #13252
>>
>> Signed-off-by: Changqing Li <changqing.li@windriver.com>
>> ---
>>   Post/0006_auto_20190917_0419.py | 24 ++++++++++++++++++++++++
>>   Post/models.py                  |  2 ++
>>   Post/parser.py                  |  2 ++
>>   Post/test.py                    |  2 ++
>>   templates/error-details.html    | 10 ++++++++++
>>   test-data/test-payload.json     |  4 +++-
>>   6 files changed, 43 insertions(+), 1 deletion(-)
>>   create mode 100644 Post/0006_auto_20190917_0419.py
>>
>> diff --git a/Post/0006_auto_20190917_0419.py b/Post/0006_auto_20190917_0419.py
>> new file mode 100644
>> index 0000000..827944e
>> --- /dev/null
>> +++ b/Post/0006_auto_20190917_0419.py
> Could you please give the migration a proper name (-n option to makemigrations) e.g. local_conf_auto_conf
OK, thanks
>
>> --- a/Post/models.py
>> +++ b/Post/models.py
>> @@ -43,6 +43,8 @@ class Build(models.Model):
>>       LINK_BACK = models.TextField(max_length=300, blank=True, null=True)
>>       ERROR_TYPE = models.CharField(max_length=20, choices=ERROR_TYPE_CHOICES,
>>                                     default=ErrorType.RECIPE)
>> +    LOCAL_CONF = models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
>> +    AUTO_CONF = models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
> I'm not sure this is practical, for two reasons:
>
> 1) Field sizes should not be variable like this; changing the MAX_UPLOAD_SIZE value after the fact would not change the database structure
> 2) The value could never actually reach MAX_UPLOAD_SIZE because the overhead of the surrounding JSON would block it from being uploaded if it did
>
> However, since this is a TextField we don't actually have to specify a max_length (for a TextField max_length only actually affects the frontend, and we don't expose this field in a form) so it can just be removed.
>
> Another thing, instead of default="" you should use blank=True.
OK,  I will fix this.
>
>
>> +        {% if detail.BUILD.LOCAL_CONF != "" %}
>> +        <dt></a>Local Conf:</dt>
>> +        <dd style="white-space: pre-wrap;">{{ detail.BUILD.LOCAL_CONF | safe }}</dd>
>> +        {% endif %}
>> +
>> +        {% if detail.BUILD.AUTO_CONF != "" %}
>> +        <dt></a>Auto Conf:</dt>
>> +        <dd style="white-space: pre-wrap;">{{ detail.BUILD.AUTO_CONF | safe }}</dd>
>> +        {% endif %}
> We cannot use the safe filter here - doing so could open up an XSS vulnerability, since anyone can upload anything to the error-report application and the content could include links or other malicious HTML data. We should allow it to be auto-escaped. Is there a particular issue you were using this to solve?

This is for resolve a problem when there is angle brackets in 
local.conf/auto.conf.

I have a patch in oe-core [OE-core] [PATCH] report-error.bbclass: 
replace angle brackets with &lt; and &gt;]

when we have below content in local.conf or auto.conf:
BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
send-error-report will fail with "HTTP Error 500: OK"

error-report-web do rudimentary check on all fields that are
passed to the graphs page to avoid any XSS happening, if contains
'<', the server will return error(Invalid characters in json).
fixed by use escape of <> to replace it.

NOTE: with this change, error-report-web need to add filter 'safe'
for the string wanted to display to avoid further HTML escaping
prior to output. Below is how the content displayed on webpage:
with the filter 'safe':
BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
without the filter 'safe':
BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj &lt;raj.khem@gmail.com&gt;"

Do you have good idea to resolve this? Thanks.

>
> Cheers
> Paul
>
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#47626): https://lists.yoctoproject.org/g/yocto/message/47626
Mute This Topic: https://lists.yoctoproject.org/mt/61340472/3617530
Group Owner: yocto+owner@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/yocto/unsub  [oe-patchwork@oe-patch.openembedded.org]
-=-=-=-=-=-=-=-=-=-=-=-
changqing.li@windriver.com Dec. 17, 2019, 3:48 a.m.
correct the mail list to yocto@lists.yoctoproject.org

On 12/11/19 1:45 PM, Changqing Li wrote:
>
> On 11/13/19 6:36 PM, Paul Eggleton wrote:
>> Hi Changqing,
>>
>> Some comments below.
>>
>> On Tuesday, 12 November 2019 9:32:53 PM NZDT 
>> changqing.li@windriver.com wrote:
>>> From: Changqing Li <changqing.li@windriver.com>
>>>
>>> Support to display local.conf and auto.conf on error report web.
>>> Here is commit in oe-core, which add local.conf/auto.conf into error 
>>> report
>>> https://git.openembedded.org/openembedded-core/commit/?id=7adf9707c04d8ef6bcd8d8bda555687f705e6ee6 
>>>
>>>
>>> This commit is related to YOCTO #13252
>>>
>>> Signed-off-by: Changqing Li <changqing.li@windriver.com>
>>> ---
>>>   Post/0006_auto_20190917_0419.py | 24 ++++++++++++++++++++++++
>>>   Post/models.py                  |  2 ++
>>>   Post/parser.py                  |  2 ++
>>>   Post/test.py                    |  2 ++
>>>   templates/error-details.html    | 10 ++++++++++
>>>   test-data/test-payload.json     |  4 +++-
>>>   6 files changed, 43 insertions(+), 1 deletion(-)
>>>   create mode 100644 Post/0006_auto_20190917_0419.py
>>>
>>> diff --git a/Post/0006_auto_20190917_0419.py 
>>> b/Post/0006_auto_20190917_0419.py
>>> new file mode 100644
>>> index 0000000..827944e
>>> --- /dev/null
>>> +++ b/Post/0006_auto_20190917_0419.py
>> Could you please give the migration a proper name (-n option to 
>> makemigrations) e.g. local_conf_auto_conf
> OK, thanks
>>
>>> --- a/Post/models.py
>>> +++ b/Post/models.py
>>> @@ -43,6 +43,8 @@ class Build(models.Model):
>>>       LINK_BACK = models.TextField(max_length=300, blank=True, 
>>> null=True)
>>>       ERROR_TYPE = models.CharField(max_length=20, 
>>> choices=ERROR_TYPE_CHOICES,
>>>                                     default=ErrorType.RECIPE)
>>> +    LOCAL_CONF = 
>>> models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
>>> +    AUTO_CONF = 
>>> models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
>> I'm not sure this is practical, for two reasons:
>>
>> 1) Field sizes should not be variable like this; changing the 
>> MAX_UPLOAD_SIZE value after the fact would not change the database 
>> structure
>> 2) The value could never actually reach MAX_UPLOAD_SIZE because the 
>> overhead of the surrounding JSON would block it from being uploaded 
>> if it did
>>
>> However, since this is a TextField we don't actually have to specify 
>> a max_length (for a TextField max_length only actually affects the 
>> frontend, and we don't expose this field in a form) so it can just be 
>> removed.
>>
>> Another thing, instead of default="" you should use blank=True.
> OK,  I will fix this.
>>
>>
>>> +        {% if detail.BUILD.LOCAL_CONF != "" %}
>>> +        <dt></a>Local Conf:</dt>
>>> +        <dd style="white-space: pre-wrap;">{{ 
>>> detail.BUILD.LOCAL_CONF | safe }}</dd>
>>> +        {% endif %}
>>> +
>>> +        {% if detail.BUILD.AUTO_CONF != "" %}
>>> +        <dt></a>Auto Conf:</dt>
>>> +        <dd style="white-space: pre-wrap;">{{ 
>>> detail.BUILD.AUTO_CONF | safe }}</dd>
>>> +        {% endif %}
>> We cannot use the safe filter here - doing so could open up an XSS 
>> vulnerability, since anyone can upload anything to the error-report 
>> application and the content could include links or other malicious 
>> HTML data. We should allow it to be auto-escaped. Is there a 
>> particular issue you were using this to solve?
>
> This is for resolve a problem when there is angle brackets in 
> local.conf/auto.conf.
>
> I have a patch in oe-core [OE-core] [PATCH] report-error.bbclass: 
> replace angle brackets with &lt; and &gt;]
>
> when we have below content in local.conf or auto.conf:
> BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
> send-error-report will fail with "HTTP Error 500: OK"
>
> error-report-web do rudimentary check on all fields that are
> passed to the graphs page to avoid any XSS happening, if contains
> '<', the server will return error(Invalid characters in json).
> fixed by use escape of <> to replace it.
>
> NOTE: with this change, error-report-web need to add filter 'safe'
> for the string wanted to display to avoid further HTML escaping
> prior to output. Below is how the content displayed on webpage:
> with the filter 'safe':
> BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
> without the filter 'safe':
> BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj &lt;raj.khem@gmail.com&gt;"
>
> Do you have good idea to resolve this? Thanks.
>
>>
>> Cheers
>> Paul
>>
>
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#47707): https://lists.yoctoproject.org/g/yocto/message/47707
Mute This Topic: https://lists.yoctoproject.org/mt/61340472/3617530
Group Owner: yocto+owner@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/yocto/unsub  [oe-patchwork@oe-patch.openembedded.org]
-=-=-=-=-=-=-=-=-=-=-=-
Khem Raj Jan. 4, 2020, 9:06 p.m.
has this resolved on server side ? since I see that
patch to report-error.bbclass is still not applied in
oe-core

On Mon, Dec 16, 2019 at 7:48 PM Changqing Li <changqing.li@windriver.com> wrote:
>
> correct the mail list to yocto@lists.yoctoproject.org
>
> On 12/11/19 1:45 PM, Changqing Li wrote:
> >
> > On 11/13/19 6:36 PM, Paul Eggleton wrote:
> >> Hi Changqing,
> >>
> >> Some comments below.
> >>
> >> On Tuesday, 12 November 2019 9:32:53 PM NZDT
> >> changqing.li@windriver.com wrote:
> >>> From: Changqing Li <changqing.li@windriver.com>
> >>>
> >>> Support to display local.conf and auto.conf on error report web.
> >>> Here is commit in oe-core, which add local.conf/auto.conf into error
> >>> report
> >>> https://git.openembedded.org/openembedded-core/commit/?id=7adf9707c04d8ef6bcd8d8bda555687f705e6ee6
> >>>
> >>>
> >>> This commit is related to YOCTO #13252
> >>>
> >>> Signed-off-by: Changqing Li <changqing.li@windriver.com>
> >>> ---
> >>>   Post/0006_auto_20190917_0419.py | 24 ++++++++++++++++++++++++
> >>>   Post/models.py                  |  2 ++
> >>>   Post/parser.py                  |  2 ++
> >>>   Post/test.py                    |  2 ++
> >>>   templates/error-details.html    | 10 ++++++++++
> >>>   test-data/test-payload.json     |  4 +++-
> >>>   6 files changed, 43 insertions(+), 1 deletion(-)
> >>>   create mode 100644 Post/0006_auto_20190917_0419.py
> >>>
> >>> diff --git a/Post/0006_auto_20190917_0419.py
> >>> b/Post/0006_auto_20190917_0419.py
> >>> new file mode 100644
> >>> index 0000000..827944e
> >>> --- /dev/null
> >>> +++ b/Post/0006_auto_20190917_0419.py
> >> Could you please give the migration a proper name (-n option to
> >> makemigrations) e.g. local_conf_auto_conf
> > OK, thanks
> >>
> >>> --- a/Post/models.py
> >>> +++ b/Post/models.py
> >>> @@ -43,6 +43,8 @@ class Build(models.Model):
> >>>       LINK_BACK = models.TextField(max_length=300, blank=True,
> >>> null=True)
> >>>       ERROR_TYPE = models.CharField(max_length=20,
> >>> choices=ERROR_TYPE_CHOICES,
> >>>                                     default=ErrorType.RECIPE)
> >>> +    LOCAL_CONF =
> >>> models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
> >>> +    AUTO_CONF =
> >>> models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
> >> I'm not sure this is practical, for two reasons:
> >>
> >> 1) Field sizes should not be variable like this; changing the
> >> MAX_UPLOAD_SIZE value after the fact would not change the database
> >> structure
> >> 2) The value could never actually reach MAX_UPLOAD_SIZE because the
> >> overhead of the surrounding JSON would block it from being uploaded
> >> if it did
> >>
> >> However, since this is a TextField we don't actually have to specify
> >> a max_length (for a TextField max_length only actually affects the
> >> frontend, and we don't expose this field in a form) so it can just be
> >> removed.
> >>
> >> Another thing, instead of default="" you should use blank=True.
> > OK,  I will fix this.
> >>
> >>
> >>> +        {% if detail.BUILD.LOCAL_CONF != "" %}
> >>> +        <dt></a>Local Conf:</dt>
> >>> +        <dd style="white-space: pre-wrap;">{{
> >>> detail.BUILD.LOCAL_CONF | safe }}</dd>
> >>> +        {% endif %}
> >>> +
> >>> +        {% if detail.BUILD.AUTO_CONF != "" %}
> >>> +        <dt></a>Auto Conf:</dt>
> >>> +        <dd style="white-space: pre-wrap;">{{
> >>> detail.BUILD.AUTO_CONF | safe }}</dd>
> >>> +        {% endif %}
> >> We cannot use the safe filter here - doing so could open up an XSS
> >> vulnerability, since anyone can upload anything to the error-report
> >> application and the content could include links or other malicious
> >> HTML data. We should allow it to be auto-escaped. Is there a
> >> particular issue you were using this to solve?
> >
> > This is for resolve a problem when there is angle brackets in
> > local.conf/auto.conf.
> >
> > I have a patch in oe-core [OE-core] [PATCH] report-error.bbclass:
> > replace angle brackets with &lt; and &gt;]
> >
> > when we have below content in local.conf or auto.conf:
> > BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
> > send-error-report will fail with "HTTP Error 500: OK"
> >
> > error-report-web do rudimentary check on all fields that are
> > passed to the graphs page to avoid any XSS happening, if contains
> > '<', the server will return error(Invalid characters in json).
> > fixed by use escape of <> to replace it.
> >
> > NOTE: with this change, error-report-web need to add filter 'safe'
> > for the string wanted to display to avoid further HTML escaping
> > prior to output. Below is how the content displayed on webpage:
> > with the filter 'safe':
> > BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
> > without the filter 'safe':
> > BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj &lt;raj.khem@gmail.com&gt;"
> >
> > Do you have good idea to resolve this? Thanks.
> >
> >>
> >> Cheers
> >> Paul
> >>
> >
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
>
> View/Reply Online (#47707): https://lists.yoctoproject.org/g/yocto/message/47707
> Mute This Topic: https://lists.yoctoproject.org/mt/61340472/1997914
> Group Owner: yocto+owner@lists.yoctoproject.org
> Unsubscribe: https://lists.yoctoproject.org/g/yocto/unsub  [raj.khem@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#47857): https://lists.yoctoproject.org/g/yocto/message/47857
Mute This Topic: https://lists.yoctoproject.org/mt/61340472/3617530
Group Owner: yocto+owner@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/yocto/unsub  [oe-patchwork@oe-patch.openembedded.org]
-=-=-=-=-=-=-=-=-=-=-=-
changqing.li@windriver.com Jan. 6, 2020, 1:02 a.m.
On 1/5/20 5:06 AM, Khem Raj wrote:
> has this resolved on server side ? since I see that
> patch to report-error.bbclass is still not applied in
> oe-core

V2 patches is for resolve this problem,  and Paul have 3 comments.

so I need to send a V3 for fix the comments.  It is easy to fix first 2 
comments,

but the last comments,  I don't have more proper way.

@Paul,  could you check my reply of comments #3?  Thanks.

>
> On Mon, Dec 16, 2019 at 7:48 PM Changqing Li <changqing.li@windriver.com> wrote:
>> correct the mail list to yocto@lists.yoctoproject.org
>>
>> On 12/11/19 1:45 PM, Changqing Li wrote:
>>> On 11/13/19 6:36 PM, Paul Eggleton wrote:
>>>> Hi Changqing,
>>>>
>>>> Some comments below.
>>>>
>>>> On Tuesday, 12 November 2019 9:32:53 PM NZDT
>>>> changqing.li@windriver.com wrote:
>>>>> From: Changqing Li <changqing.li@windriver.com>
>>>>>
>>>>> Support to display local.conf and auto.conf on error report web.
>>>>> Here is commit in oe-core, which add local.conf/auto.conf into error
>>>>> report
>>>>> https://git.openembedded.org/openembedded-core/commit/?id=7adf9707c04d8ef6bcd8d8bda555687f705e6ee6
>>>>>
>>>>>
>>>>> This commit is related to YOCTO #13252
>>>>>
>>>>> Signed-off-by: Changqing Li <changqing.li@windriver.com>
>>>>> ---
>>>>>    Post/0006_auto_20190917_0419.py | 24 ++++++++++++++++++++++++
>>>>>    Post/models.py                  |  2 ++
>>>>>    Post/parser.py                  |  2 ++
>>>>>    Post/test.py                    |  2 ++
>>>>>    templates/error-details.html    | 10 ++++++++++
>>>>>    test-data/test-payload.json     |  4 +++-
>>>>>    6 files changed, 43 insertions(+), 1 deletion(-)
>>>>>    create mode 100644 Post/0006_auto_20190917_0419.py
>>>>>
>>>>> diff --git a/Post/0006_auto_20190917_0419.py
>>>>> b/Post/0006_auto_20190917_0419.py
>>>>> new file mode 100644
>>>>> index 0000000..827944e
>>>>> --- /dev/null
>>>>> +++ b/Post/0006_auto_20190917_0419.py
>>>> Could you please give the migration a proper name (-n option to
>>>> makemigrations) e.g. local_conf_auto_conf
>>> OK, thanks
>>>>> --- a/Post/models.py
>>>>> +++ b/Post/models.py
>>>>> @@ -43,6 +43,8 @@ class Build(models.Model):
>>>>>        LINK_BACK = models.TextField(max_length=300, blank=True,
>>>>> null=True)
>>>>>        ERROR_TYPE = models.CharField(max_length=20,
>>>>> choices=ERROR_TYPE_CHOICES,
>>>>>                                      default=ErrorType.RECIPE)
>>>>> +    LOCAL_CONF =
>>>>> models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
>>>>> +    AUTO_CONF =
>>>>> models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
>>>> I'm not sure this is practical, for two reasons:
>>>>
>>>> 1) Field sizes should not be variable like this; changing the
>>>> MAX_UPLOAD_SIZE value after the fact would not change the database
>>>> structure
>>>> 2) The value could never actually reach MAX_UPLOAD_SIZE because the
>>>> overhead of the surrounding JSON would block it from being uploaded
>>>> if it did
>>>>
>>>> However, since this is a TextField we don't actually have to specify
>>>> a max_length (for a TextField max_length only actually affects the
>>>> frontend, and we don't expose this field in a form) so it can just be
>>>> removed.
>>>>
>>>> Another thing, instead of default="" you should use blank=True.
>>> OK,  I will fix this.
>>>>
>>>>> +        {% if detail.BUILD.LOCAL_CONF != "" %}
>>>>> +        <dt></a>Local Conf:</dt>
>>>>> +        <dd style="white-space: pre-wrap;">{{
>>>>> detail.BUILD.LOCAL_CONF | safe }}</dd>
>>>>> +        {% endif %}
>>>>> +
>>>>> +        {% if detail.BUILD.AUTO_CONF != "" %}
>>>>> +        <dt></a>Auto Conf:</dt>
>>>>> +        <dd style="white-space: pre-wrap;">{{
>>>>> detail.BUILD.AUTO_CONF | safe }}</dd>
>>>>> +        {% endif %}
>>>> We cannot use the safe filter here - doing so could open up an XSS
>>>> vulnerability, since anyone can upload anything to the error-report
>>>> application and the content could include links or other malicious
>>>> HTML data. We should allow it to be auto-escaped. Is there a
>>>> particular issue you were using this to solve?
>>> This is for resolve a problem when there is angle brackets in
>>> local.conf/auto.conf.
>>>
>>> I have a patch in oe-core [OE-core] [PATCH] report-error.bbclass:
>>> replace angle brackets with &lt; and &gt;]
>>>
>>> when we have below content in local.conf or auto.conf:
>>> BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
>>> send-error-report will fail with "HTTP Error 500: OK"
>>>
>>> error-report-web do rudimentary check on all fields that are
>>> passed to the graphs page to avoid any XSS happening, if contains
>>> '<', the server will return error(Invalid characters in json).
>>> fixed by use escape of <> to replace it.
>>>
>>> NOTE: with this change, error-report-web need to add filter 'safe'
>>> for the string wanted to display to avoid further HTML escaping
>>> prior to output. Below is how the content displayed on webpage:
>>> with the filter 'safe':
>>> BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
>>> without the filter 'safe':
>>> BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj &lt;raj.khem@gmail.com&gt;"
>>>
>>> Do you have good idea to resolve this? Thanks.
>>>
>>>> Cheers
>>>> Paul
>>>>
>> -=-=-=-=-=-=-=-=-=-=-=-
>> Links: You receive all messages sent to this group.
>>
>> View/Reply Online (#47707): https://lists.yoctoproject.org/g/yocto/message/47707
>> Mute This Topic: https://lists.yoctoproject.org/mt/61340472/1997914
>> Group Owner: yocto+owner@lists.yoctoproject.org
>> Unsubscribe: https://lists.yoctoproject.org/g/yocto/unsub  [raj.khem@gmail.com]
>> -=-=-=-=-=-=-=-=-=-=-=-
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#47872): https://lists.yoctoproject.org/g/yocto/message/47872
Mute This Topic: https://lists.yoctoproject.org/mt/61340472/3617530
Group Owner: yocto+owner@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/yocto/unsub  [oe-patchwork@oe-patch.openembedded.org]
-=-=-=-=-=-=-=-=-=-=-=-
changqing.li@windriver.com Feb. 14, 2020, 2:42 a.m.
Hi, Paul

Could you help to check my reply below, thanks.

On 12/11/19 1:45 PM, Changqing Li wrote:
>
> On 11/13/19 6:36 PM, Paul Eggleton wrote:
>> Hi Changqing,
>>
>> Some comments below.
>>
>> On Tuesday, 12 November 2019 9:32:53 PM NZDT 
>> changqing.li@windriver.com wrote:
>>> From: Changqing Li <changqing.li@windriver.com>
>>>
>>> Support to display local.conf and auto.conf on error report web.
>>> Here is commit in oe-core, which add local.conf/auto.conf into error 
>>> report
>>> https://git.openembedded.org/openembedded-core/commit/?id=7adf9707c04d8ef6bcd8d8bda555687f705e6ee6 
>>>
>>>
>>> This commit is related to YOCTO #13252
>>>
>>> Signed-off-by: Changqing Li <changqing.li@windriver.com>
>>> ---
>>>   Post/0006_auto_20190917_0419.py | 24 ++++++++++++++++++++++++
>>>   Post/models.py                  |  2 ++
>>>   Post/parser.py                  |  2 ++
>>>   Post/test.py                    |  2 ++
>>>   templates/error-details.html    | 10 ++++++++++
>>>   test-data/test-payload.json     |  4 +++-
>>>   6 files changed, 43 insertions(+), 1 deletion(-)
>>>   create mode 100644 Post/0006_auto_20190917_0419.py
>>>
>>> diff --git a/Post/0006_auto_20190917_0419.py 
>>> b/Post/0006_auto_20190917_0419.py
>>> new file mode 100644
>>> index 0000000..827944e
>>> --- /dev/null
>>> +++ b/Post/0006_auto_20190917_0419.py
>> Could you please give the migration a proper name (-n option to 
>> makemigrations) e.g. local_conf_auto_conf
> OK, thanks
>>
>>> --- a/Post/models.py
>>> +++ b/Post/models.py
>>> @@ -43,6 +43,8 @@ class Build(models.Model):
>>>       LINK_BACK = models.TextField(max_length=300, blank=True, 
>>> null=True)
>>>       ERROR_TYPE = models.CharField(max_length=20, 
>>> choices=ERROR_TYPE_CHOICES,
>>>                                     default=ErrorType.RECIPE)
>>> +    LOCAL_CONF = 
>>> models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
>>> +    AUTO_CONF = 
>>> models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
>> I'm not sure this is practical, for two reasons:
>>
>> 1) Field sizes should not be variable like this; changing the 
>> MAX_UPLOAD_SIZE value after the fact would not change the database 
>> structure
>> 2) The value could never actually reach MAX_UPLOAD_SIZE because the 
>> overhead of the surrounding JSON would block it from being uploaded 
>> if it did
>>
>> However, since this is a TextField we don't actually have to specify 
>> a max_length (for a TextField max_length only actually affects the 
>> frontend, and we don't expose this field in a form) so it can just be 
>> removed.
>>
>> Another thing, instead of default="" you should use blank=True.
> OK,  I will fix this.
>>
>>
>>> +        {% if detail.BUILD.LOCAL_CONF != "" %}
>>> +        <dt></a>Local Conf:</dt>
>>> +        <dd style="white-space: pre-wrap;">{{ 
>>> detail.BUILD.LOCAL_CONF | safe }}</dd>
>>> +        {% endif %}
>>> +
>>> +        {% if detail.BUILD.AUTO_CONF != "" %}
>>> +        <dt></a>Auto Conf:</dt>
>>> +        <dd style="white-space: pre-wrap;">{{ 
>>> detail.BUILD.AUTO_CONF | safe }}</dd>
>>> +        {% endif %}
>> We cannot use the safe filter here - doing so could open up an XSS 
>> vulnerability, since anyone can upload anything to the error-report 
>> application and the content could include links or other malicious 
>> HTML data. We should allow it to be auto-escaped. Is there a 
>> particular issue you were using this to solve?
>
> This is for resolve a problem when there is angle brackets in 
> local.conf/auto.conf.
>
> I have a patch in oe-core [OE-core] [PATCH] report-error.bbclass: 
> replace angle brackets with &lt; and &gt;]
>
> when we have below content in local.conf or auto.conf:
> BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
> send-error-report will fail with "HTTP Error 500: OK"
>
> error-report-web do rudimentary check on all fields that are
> passed to the graphs page to avoid any XSS happening, if contains
> '<', the server will return error(Invalid characters in json).
> fixed by use escape of <> to replace it.
>
> NOTE: with this change, error-report-web need to add filter 'safe'
> for the string wanted to display to avoid further HTML escaping
> prior to output. Below is how the content displayed on webpage:
> with the filter 'safe':
> BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
> without the filter 'safe':
> BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj &lt;raj.khem@gmail.com&gt;"
>
> Do you have good idea to resolve this? Thanks.
>
>>
>> Cheers
>> Paul
>>
>
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#48405): https://lists.yoctoproject.org/g/yocto/message/48405
Mute This Topic: https://lists.yoctoproject.org/mt/61340472/3617530
Group Owner: yocto+owner@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/yocto/unsub  [oe-patchwork@oe-patch.openembedded.org]
-=-=-=-=-=-=-=-=-=-=-=-
Armin Kuster Feb. 14, 2020, 4:18 p.m.
Changqing,

Please use bluelightning@bluelightning.org for now.

According to Linkedin, Paul is now at Microsoft.

- armin


On 2/13/20 6:42 PM, Changqing Li wrote:
> Hi, Paul
>
> Could you help to check my reply below, thanks.
>
> On 12/11/19 1:45 PM, Changqing Li wrote:
>>
>> On 11/13/19 6:36 PM, Paul Eggleton wrote:
>>> Hi Changqing,
>>>
>>> Some comments below.
>>>
>>> On Tuesday, 12 November 2019 9:32:53 PM NZDT
>>> changqing.li@windriver.com wrote:
>>>> From: Changqing Li <changqing.li@windriver.com>
>>>>
>>>> Support to display local.conf and auto.conf on error report web.
>>>> Here is commit in oe-core, which add local.conf/auto.conf into
>>>> error report
>>>> https://git.openembedded.org/openembedded-core/commit/?id=7adf9707c04d8ef6bcd8d8bda555687f705e6ee6
>>>>
>>>>
>>>> This commit is related to YOCTO #13252
>>>>
>>>> Signed-off-by: Changqing Li <changqing.li@windriver.com>
>>>> ---
>>>>   Post/0006_auto_20190917_0419.py | 24 ++++++++++++++++++++++++
>>>>   Post/models.py                  |  2 ++
>>>>   Post/parser.py                  |  2 ++
>>>>   Post/test.py                    |  2 ++
>>>>   templates/error-details.html    | 10 ++++++++++
>>>>   test-data/test-payload.json     |  4 +++-
>>>>   6 files changed, 43 insertions(+), 1 deletion(-)
>>>>   create mode 100644 Post/0006_auto_20190917_0419.py
>>>>
>>>> diff --git a/Post/0006_auto_20190917_0419.py
>>>> b/Post/0006_auto_20190917_0419.py
>>>> new file mode 100644
>>>> index 0000000..827944e
>>>> --- /dev/null
>>>> +++ b/Post/0006_auto_20190917_0419.py
>>> Could you please give the migration a proper name (-n option to
>>> makemigrations) e.g. local_conf_auto_conf
>> OK, thanks
>>>
>>>> --- a/Post/models.py
>>>> +++ b/Post/models.py
>>>> @@ -43,6 +43,8 @@ class Build(models.Model):
>>>>       LINK_BACK = models.TextField(max_length=300, blank=True,
>>>> null=True)
>>>>       ERROR_TYPE = models.CharField(max_length=20,
>>>> choices=ERROR_TYPE_CHOICES,
>>>>                                     default=ErrorType.RECIPE)
>>>> +    LOCAL_CONF =
>>>> models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
>>>> +    AUTO_CONF =
>>>> models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
>>> I'm not sure this is practical, for two reasons:
>>>
>>> 1) Field sizes should not be variable like this; changing the
>>> MAX_UPLOAD_SIZE value after the fact would not change the database
>>> structure
>>> 2) The value could never actually reach MAX_UPLOAD_SIZE because the
>>> overhead of the surrounding JSON would block it from being uploaded
>>> if it did
>>>
>>> However, since this is a TextField we don't actually have to specify
>>> a max_length (for a TextField max_length only actually affects the
>>> frontend, and we don't expose this field in a form) so it can just
>>> be removed.
>>>
>>> Another thing, instead of default="" you should use blank=True.
>> OK,  I will fix this.
>>>
>>>
>>>> +        {% if detail.BUILD.LOCAL_CONF != "" %}
>>>> +        <dt></a>Local Conf:</dt>
>>>> +        <dd style="white-space: pre-wrap;">{{
>>>> detail.BUILD.LOCAL_CONF | safe }}</dd>
>>>> +        {% endif %}
>>>> +
>>>> +        {% if detail.BUILD.AUTO_CONF != "" %}
>>>> +        <dt></a>Auto Conf:</dt>
>>>> +        <dd style="white-space: pre-wrap;">{{
>>>> detail.BUILD.AUTO_CONF | safe }}</dd>
>>>> +        {% endif %}
>>> We cannot use the safe filter here - doing so could open up an XSS
>>> vulnerability, since anyone can upload anything to the error-report
>>> application and the content could include links or other malicious
>>> HTML data. We should allow it to be auto-escaped. Is there a
>>> particular issue you were using this to solve?
>>
>> This is for resolve a problem when there is angle brackets in
>> local.conf/auto.conf.
>>
>> I have a patch in oe-core [OE-core] [PATCH] report-error.bbclass:
>> replace angle brackets with &lt; and &gt;]
>>
>> when we have below content in local.conf or auto.conf:
>> BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
>> send-error-report will fail with "HTTP Error 500: OK"
>>
>> error-report-web do rudimentary check on all fields that are
>> passed to the graphs page to avoid any XSS happening, if contains
>> '<', the server will return error(Invalid characters in json).
>> fixed by use escape of <> to replace it.
>>
>> NOTE: with this change, error-report-web need to add filter 'safe'
>> for the string wanted to display to avoid further HTML escaping
>> prior to output. Below is how the content displayed on webpage:
>> with the filter 'safe':
>> BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
>> without the filter 'safe':
>> BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj &lt;raj.khem@gmail.com&gt;"
>>
>> Do you have good idea to resolve this? Thanks.
>>
>>>
>>> Cheers
>>> Paul
>>>
>>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#48422): https://lists.yoctoproject.org/g/yocto/message/48422
Mute This Topic: https://lists.yoctoproject.org/mt/61340472/3617530
Group Owner: yocto+owner@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/yocto/unsub  [oe-patchwork@oe-patch.openembedded.org]
-=-=-=-=-=-=-=-=-=-=-=-
Richard Purdie March 22, 2021, 5:57 p.m.
On Mon, 2021-03-22 at 15:32 +0000, Richard Purdie via lists.yoctoproject.org wrote:
> Sorry about the delay on this, we do really need to get this resolved.
> I'm wondering if we should replace the angled brackets test with
> https://github.com/mozilla/bleach which would then remove the need
> for these workarounds.
> 
> Would you be able to update the patch for the others issues please
> and then we can look at this one separately?

I just sent out an as yet untested patch which may fix some of the quoting
issues by using bleach. I'd still need to add it to the requirements file...

Cheers,

Richard
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#52800): https://lists.yoctoproject.org/g/yocto/message/52800
Mute This Topic: https://lists.yoctoproject.org/mt/61340472/3617530
Group Owner: yocto+owner@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/yocto/unsub [oe-patchwork@oe-patch.openembedded.org]
-=-=-=-=-=-=-=-=-=-=-=-