Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/415/ for more details.

My main issue on that change is that the generated links will not be available anymore in the displayed error message, see output below after your change on that example (https://archive.softwareheritage.org/browse/revision/041f5ea1cf987a4068ef5f39ba0a09be85952064/?origin=https://github.com/git/got):

swh.web.common.exc.NotFoundExc: The Software Heritage archive has a revision with the hash you provided but the origin mentioned in your request appears broken: <a href="https://github.com/git/got">https://github.com/git/got</a>. Please check the URL and try again.

Nevertheless, you can still browse the revision without origin information: <a href="/browse/revision/041f5ea1cf987a4068ef5f39ba0a09be85952064/">/browse/revision/041f5ea1cf987a4068ef5f39ba0a09be85952064/</a>

This makes the navigation more painful as the user will have to copy / paste the link instead of simply clicking on it.

Considering the only vulnerability here is if someone submits an origin url through a save code now request containing <script> tags in it, I think it will be better to filter out that kind
of bogus data directly in the common/origin_save.py file.

@anlambert
Ok. Will need to rework this, to keep the links working.
But,
1- if we filter out directly while saving, it may cause some valid links being rejected.
2- if I use filterXSS, other html tags will still work.

@kalpitk , indeed this is not so easy as it seems to fix any possible XSS injections.

1- if we filter out directly while saving, it may cause some valid links being rejected.

Yes, this calls for clever filtering server-side on the submitted origin urls to save into the archive.

2- if I use filterXSS, other html tags will still work.

Maybe but at least the harmful tags will be removed so this is not really a big deal IMHO.

We should really add some end to end tests using Selenium in a near future to better detect
and prevent XSS vulnerabilities.

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/421/ for more details.

@anlambert We need to remember to use 'escape' unsafe things, whenever marking some html as safe.

I think, I'll add selenium end-to-end tests in another commit (as per Timeline in my GSoC proposal?)

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/422/ for more details.

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/424/ for more details.

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/425/ for more details.

@anlambert We need to remember to use 'escape' unsafe things, whenever marking some html as safe.

Indeed, can you create a task on the forge to track this work ?

I think, I'll add selenium end-to-end tests in another commit (as per Timeline in my GSoC proposal?)

Of course, adding end-to-end tests should be handled in a separate diff and a task should also be created on the forge (maybe I already created one, need to check that).

@kalpitk , I found other XSS vulnerabilities related to error pages. In the file common/exc.py, request.META['PATH_INFO'] values must also be escaped (see https://forge.softwareheritage.org/source/swh-web/browse/master/swh/web/common/exc.py$71 as an example).

Also, every message set to the following exception types: NotFoundExc, BadInpuExc, ForbiddenExc in the browse code should be checked for url escaping (see https://forge.softwareheritage.org/source/swh-web/browse/master/swh/web/browse/views/utils/snapshot_context.py$80-120 for instance)

Can you add these extra url escaping to that diff ?

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/426/ for more details.

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/427/ for more details.

@anlambert I think, I have added escape wherever it was vulnerable.

checksums and ids and plain strings can't have xss vulnerability.

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/428/ for more details.

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/429/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/429/console

@kalpitk , I found where the XSS vulnerability is located when a branch name contains some HTML code in it.

In the file templates/includes/top-navigation.html, you need to replace:

var snapshotContext = false;
var branch = false;
{% if snapshot_context %}
  snapshotContext = true;
  branch = {{ snapshot_context.branch|jsonify }};
{% endif %}
swh.browse.initSnapshotNavigation(snapshotContext, branch);

by

var snapshotContext = false;
var branch = false;
{% if snapshot_context %}
  snapshotContext = true;
  branch = "{{ snapshot_context.branch|escape }}";
{% endif %}
swh.browse.initSnapshotNavigation(snapshotContext, branch !== "None");

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/430/ for more details.

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/431/ for more details.