Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: escape user input to prevent script injection #56

Conversation

LayZeeDK
Copy link
Contributor

@LayZeeDK LayZeeDK commented Aug 31, 2022

Security hardening

  • Use the core API for reading input parameters to remove dependency on argument order in the Node.js script
  • Escape user inputs in shell commands to prevent script injections
  • Pass the GitHub token as an environment variable instead of a command line argument to decrease the risk of token leaks
  • Use the core API for logging to decrease the risk of secret leaks

@LayZeeDK LayZeeDK marked this pull request as ready for review August 31, 2022 23:50
@LayZeeDK LayZeeDK force-pushed the LayZeeDK/refactor/escape-input-to-prevent-script-injection branch 2 times, most recently from 2a277ec to fbeeef0 Compare September 1, 2022 09:48
@LayZeeDK LayZeeDK mentioned this pull request Sep 1, 2022
@LayZeeDK LayZeeDK force-pushed the LayZeeDK/refactor/escape-input-to-prevent-script-injection branch from d7d3012 to cf45931 Compare September 1, 2022 20:59
@meeroslav meeroslav merged commit c1bcacc into nrwl:main Sep 2, 2022
@LayZeeDK LayZeeDK deleted the LayZeeDK/refactor/escape-input-to-prevent-script-injection branch September 2, 2022 11:34
JamesHenry added a commit that referenced this pull request Sep 2, 2022
JamesHenry added a commit that referenced this pull request Sep 2, 2022
@JamesHenry
Copy link
Collaborator

JamesHenry commented Sep 2, 2022

I have reverted this because it caused a lot of folks to break, please take a look at their comments on #59

meeroslav pushed a commit to meeroslav/nx-set-shas that referenced this pull request Oct 26, 2022
* refactor: use run step condition (nrwl#55)

* refactor: use run step condition

Composite run steps have had support for conditions [since November 9th, 2021](https://github.blog/changelog/2021-11-09-github-actions-conditional-execution-of-steps-in-actions/).

* chore: bump patch version

* refactor: escape user inputs to prevent script injections

See [Understanding the risk of script injections](https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#understanding-the-risk-of-script-injections).

* refactor: pass GitHub token as environment variable instead of command line argument

This is done to decrease the change of the GitHub token from appearing in any logs.

* refactor: use core API for logging to decrease the risk of secret leaks

* chore: bump patch version

(cherry picked from commit c1bcacc)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants