Repository navigation
Unexpected token #5
Description
There appears to be something dodgy going on when PR Badge processes JavaScript or injects output into the badge. This was made evident when I attempted to do some more advanced JavaScript processing of the payload data:
- label: Howdy
message: "$payload.pull_request.labels.filter(label => label.name.toLowerCase().includes('blocked')).map(label => label.name.replace(/blocked /i, '')).join(', ')"
when: "$payload.pull_request.labels.filter(label => label.name.toLowerCase().includes('blocked'))"
The result is the following badge in my PR:
'/red) [<img width="16" alt="Powered by Pull Request Badge" src="https://user-images.githubusercontent.com/1393946/111216524-d2bb8e00-85d4-11eb-821b-ed4c00989c02.png">](https://pullrequestbadge.com/?utm_medium=github&utm_source=NetsoftHoldings&utm_campaign=badge_info)<!-- PR-BADGE: PLEASE DO NOT REMOVE THIS COMMENT -->
'/red)
- PR Badge could have a dedicated facility for surfacing errors that occur during processing in a nice and clean way.
- PR Badge should probably ensure it correctly treats the input and output data at the correct level of injection and using the correct degree of escaping to avoid causing poor JavaScript to be generated or worse, permitting an unexpected degree of code injection. I suspect there are data injection issues at multiple levels going on here at the same time.
- It would be nice if there were facilities for dumping full objects, such as the
$payload, for testing, but also to display or pass JSON on to eg. web requests. Ideally amessage: $payloadwould just show me the full payload as a JSON string without any hoopla.
Activity
stefanbuck commentedon Mar 4, 2023
A lot of great ideas in this single issue, so thank you for taking the time to write them down. I'll have a look at the actual issue and report back soon.
stefanbuck commentedon Mar 5, 2023
Agree, any suggestions what would be the best interface for this? The PR itself or something external? How about adding this capability to the playground itself?
Right now just the
whenfield is evaluated as JavaScript and the output of this needs to be a boolean. All the other fields support a very opinionated implementation (string interpolation) to allow users to mix strings and some basic JavaScript. Changing this behaviour while keeping backwards compatibility is quite tricky maybe even impossible. For example, how do I differentiate between1 + 1being just a string or a JavaScript operation?The current solution is quite robust with regards of code injections. Retaining this is a key requirement.
Good idea, but I wonder if this could leak sensitive information when shown on a PR itself. Maybe best to tweak the playground so you can specify custom payload if needed, or run it against a public PR.
lhunath commentedon May 10, 2023
Have you considered open-sourcing the implementation? I'd be able to help if I could look at what parsing you're performing or make suggestions.
lhunath commentedon Jan 24, 2024
I could really benefit from a better understanding of the syntax concerns involved in authoring
whenstrings. Really all we've got to go on is:But in reality, there is clearly a lot more going on. Are variable values treated as actual JavaScript variables or are they injected prior to evaluation? How does quoting work? Can I use variables inside regex matches? Etc.
The lack of clarity on this is forcing me to make tons of commits throwing random syntax attempts at it until something hopefully stops mysteriously failing.
For instance, can I do? Seems like no? Why?