Skip to content

[19.01] Improve string escaping and changeset validation on toolshed#7616

Merged
nsoranzo merged 4 commits intogalaxyproject:release_19.01from
martenson:ts-xss
Mar 29, 2019
Merged

[19.01] Improve string escaping and changeset validation on toolshed#7616
nsoranzo merged 4 commits intogalaxyproject:release_19.01from
martenson:ts-xss

Conversation

@martenson
Copy link
Member

No description provided.

@nsoranzo
Copy link
Member

@martenson
Copy link
Member Author

the previous similar PR from January went to 18.09, but I am not against backporting

@jmchilton
Copy link
Member

We shouldn't support tool sheds and haven't really promised to anywhere - let alone 18.05 tool sheds - I feel like targeting 19.01 is sufficient.

@nsoranzo
Copy link
Member

@jmchilton Since we release the Tool Shed source code together with Galaxy's, I don't see how the Security Policy doesn't apply to it. Obviously we could amend the policy, if we decide to.

@jmchilton
Copy link
Member

The security policy over and over mention's Galaxy's version, Galaxy instances, etc... it doesn't mention "code" or "code shipped with Galaxy" or alternative servers, or the Tool Shed. I think my reading of it is entirely reasonable.

@nsoranzo
Copy link
Member

There has never been a separate Tool Shed repository or separate Tool Shed releases, Galaxy source code imports TS and viceversa. This clearly indicates to me that "Galaxy" in the Sec. policy refers to the entire "project".

@martenson
Copy link
Member Author

Unless there are big obstacles/conflicts I volunteer to backport it.

@nsoranzo
Copy link
Member

@martenson I've fixed the tests, can you double check that this still fix the original issue?

@martenson
Copy link
Member Author

from gitter

@nsoranzo I suspect there are more than 3 statuses
we might use 'warning'?
and I don't think we use 'info'
info seems like a reasonable default though, so let's just add warning?

which will not strip safe HTML tags and attributes, which we use.

Also, restrict the possible values of `status` instead of trying to
escape it.
@nsoranzo
Copy link
Member

Thanks @martenson , I've rebased the latest commit adding warning.

@martenson
Copy link
Member Author

@nsoranzo bleach seems to work well in my testing of these endpoints. This is ready I think.

@nsoranzo nsoranzo merged commit 0f80279 into galaxyproject:release_19.01 Mar 29, 2019
@nsoranzo nsoranzo deleted the ts-xss branch March 29, 2019 17:46
@nsoranzo
Copy link
Member

Merged forward.

@martenson
Copy link
Member Author

backport: #7626

@martenson martenson mentioned this pull request Mar 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants