treeverse
Pull Requests: Sanitize to user provided fields
We should sanitize any user inputted fields in pull requests such as Title and Description to avoid potentially malicious attacks
In case this is not part of the plan: It's cool to do it on the backend, but please (also) protect when inserting the user provided fields into the DOM. So it really needs to be based on an allowlist rather than on a blocklist. That list can come (and probably should) from an existing React library, of course.
@itaigilo I think this is something we can do this current sprint. WDYT?
Yeah, I guess so. Will do it on my next workday.
@itaigilo should we close this?
@N-o-Z can we close it?
There's an open PR linked to this issue - what about it?
There's an open PR linked to this issue - what about it?
It's not relevant anymore - there's no need to sanitize the fields on the UI side, and this PR can be closed.
Why we do not believe that there is a threat here. lakeFS handles user-created metadata all over the place. For instance, a commit message is considerably more sensitive than a PR description because it appears in more places. We handle storing and displaying metadata by programming correctly: use the KV library, use pgx prepared queries when accessing SQL, using React to display data.
How this works, with examples from PRs>
- WebUI uses React components. React components handle HTML-sensitive content correctly. For instance this is how we currently display description.
- The backend uses KV to read and write data objects. For instance this is how we write a PR to KV.
So there is no issue here.