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

Filter NODE_OPTIONS from env for file output #2775

Merged
merged 9 commits into from
Aug 16, 2023

Conversation

cory-miller
Copy link
Contributor

@cory-miller cory-miller commented Aug 16, 2023

This matches the deprecated command equivalent as seen here.

@cory-miller cory-miller changed the title Users/cory miller/heredoc bug 2 Filter NODE_OPTIONS from env for file output Aug 16, 2023
@cory-miller cory-miller marked this pull request as ready for review August 16, 2023 18:27
@cory-miller cory-miller requested a review from a team as a code owner August 16, 2023 18:27
@cory-miller cory-miller merged commit 8b9a81c into main Aug 16, 2023
10 checks passed
@cory-miller cory-miller deleted the users/cory-miller/heredoc-bug-2 branch August 16, 2023 21:56
@rymndhng
Copy link

@cory-miller @TingluoHuang I started seeing these errors on our worker (we set NODE_OPTIONS by writing to $GITHUB_ENV).

Can you clarify why setting this envvar is not allowed?

@synaptiko
Copy link

synaptiko commented Sep 12, 2023

@cory-miller Our whole CI pipeline is failing since today morning due to this. We need to increase Node.js's memory and without it our commands are failing on OOMs.

Up until this point setting NODE_OPTIONS worked fine. We use this:

echo "NODE_OPTIONS=--max-old-space-size=${MAX_OLDSPACE_SIZE_MB}" >> $GITHUB_ENV

I checked both PRs related to blocking NODE_OPTIONS but there is no explanation why it's blocked and what is an alternative way to set it. Can you please shed some light on this topic? I tried to google it as well but no luck.

The problem is that our deployments are blocked by this quite hidden change. And it took us a while to find this PR as well.

There is no note about NODE_OPTIONS being special in the official docs either: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-an-environment-variable

cc @pleszkowicz @icaliskanoglu

@cory-miller
Copy link
Contributor Author

For the deprecated set-env command we received bug bounty reports showing exploits of arbitrary code execution using NODE_OPTIONS. This led to the initial filtering for set-env. When introducing GITHUB_ENV for some reason this filter was added despite also being vulnerable to the initial report. This led to subsequent reports related to arbitrary code execution hence the need to add back the filtering.

I will look at the docs and update them.

byCedric added a commit to expo/snack that referenced this pull request Sep 17, 2023
…to website and snackager"

This reverts commit d0b8c0b.

Github does not allow setting `NODE_OPTIONS` through `$GITHUB_ENV`.

See: actions/runner#2775
byCedric added a commit to expo/snack that referenced this pull request Sep 17, 2023
This reverts commit 3cbbcaf.

GitHub does not allow setting `NODE_OPTIONS` through `$GITHUB_ENV`.

See: actions/runner#2775
byCedric added a commit to expo/snack that referenced this pull request Sep 18, 2023
This reverts commit 3cbbcaf.

GitHub does not allow setting `NODE_OPTIONS` through `$GITHUB_ENV`.

See: actions/runner#2775
byCedric added a commit to expo/snack that referenced this pull request Sep 18, 2023
* refactor: upgrade workflows to node `18.17.1`

* refactor: upgrade skaffold to node `18.17.1`

* refactor: upgrade volta to node `18.17.1`

* fix: add `NODE_OPTIONS=--openssl-legacy-provider` workaround to website and snackager

* refactor: clean up `NODE_OPTIONS` workaround in workflows

* Revert "refactor: clean up `NODE_OPTIONS` workaround in workflows"

This reverts commit 3cbbcaf.

GitHub does not allow setting `NODE_OPTIONS` through `$GITHUB_ENV`.

See: actions/runner#2775
@georgehb
Copy link

georgehb commented Oct 9, 2023

For anyone looking for a workaround, you can set the value as an output parameter and then set it as the env for each step that requires it.

eg

steps:
  - name: Calculate NODE_OPTIONS
    id: node_ops
    run: |
      MAX_OLDSPACE_SIZE_MB=8192
      echo "val=--max-old-space-size=${MAX_OLDSPACE_SIZE_MB}" >> "$GITHUB_OUTPUT"
  - name: Tests
    run: npm run test
    env:
      NODE_OPTIONS: ${{steps.node_ops.outputs.val}}

@vlkl-sap
Copy link

The workaround works. What is the threat model here?

@eluchsinger
Copy link

This workaround is not working for me anymore... :/

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.

7 participants