fix: do not test for il2cpp that does not exist in 2019.2 or earlier

[mob-sakai]

Changes

• fix grep command to check Unity version (2019.3 or later)
• For 2019.2 or earlier, use base image to test

Checklist

• Read the contribution guide and accept the code of conduct
• Readme (updated or not needed)

[mob-sakai]

This PR fixes some tests.
https://github.com/game-ci/docker/actions/runs/499502896

Version	Module	Fixed?
2019.2.21f1	base	❌ -> ✅
2019.1.14f1	base	❌ -> ✅
2018.4.30f1	base	❌ -> ✅
2018.3.14f1	base	❌ -> ✅

[mob-sakai]

Hmm, when the test workflow was triggered via PR, secrets.UNITY_LICENSE will be empty.
https://github.com/game-ci/docker/runs/1738698265?check_suite_focus=true

NOTE: Correct output: UNITY_LICENSE: *** (Triggered via push).
https://github.com/game-ci/docker/runs/1737594577?check_suite_focus=true

[mob-sakai]

We need to embed the Unity license file in the workflow, like unity-test-runner or unity-builder.

https://github.com/game-ci/unity-test-runner/blob/main/.github/workflows/main.yml#L7
https://github.com/game-ci/unity-builder/blob/main/.github/workflows/main.yml#L111

EDIT:

For example:

env:
  UNITY_LICENSE: "<?xml version=\"1.0\" encoding=\"UTF-8\"?><root>\n ... </root>"

at f893bd3#diff-faff1af3d8ff408964a57b2e475f69a6b7c7b71c9978cccc8f471798caac2c88L94-R95

NOTE: The above embed Unity license file will give an error because the machine ID is different.
576562626572264761624c65526f7578 != d39b8e2f4d364b2e98b06afa0c6e08c5

[webbertakken]

Yea you're right. This is because the variable is not exposed for external PRs (or pushes for that matter).

I'll read this post about allowing secrets for public PRs tonight and fix it or use your suggested approach.

[webbertakken]

@mob-sakai could you please rebase or pull main into your branch? It should fix the problem.

See #66 for the changes.

[mob-sakai]

Rebased on main.

[mob-sakai]

Thanks.
The tests were run with a secret UNITY_LICENSE, but the workflow was from the main branch (i.e. without merge commit).
So it's not actually a test for this PR.

[mob-sakai]

https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/

However, instead of running against the workflow and code from the merge commit, the event runs against the workflow and code from the base of the pull request.

This means that merge commits will not be tested even if PR is created that changes Dockerfile.
So we can check out the merge commit as follows.

- uses: actions/checkout@v2
  with:
    ref: ${{github.event.pull_request.head.ref}}
    repository: ${{github.event.pull_request.head.repo.full_name}}

However, "the changed workflow in merge commit" will be not tested, to prevent to run malicious workflows.
For example, if a PR is created that contains the following malicious workflow, it is safe until merged.

- run: |
    # Some malicious code with '$PASSWORD'
    ...
  env:
    PASSWORD: ${{ secrets.PASSWORD }}

[mob-sakai]

If you hope to test the workflow (test.yml) changes, you need to embed the Unity license file in the workflow.
(And revert #66)

[webbertakken]

Yea that's a limitation of pull_request_target unfortunately. But at least it will work for PRs that don't change the workflow itself. Which I expect would be most of the changes.

This means that merge commits will not be tested even if PR is created that changes Dockerfile.
So we can check out the merge commit as follows.

- uses: actions/checkout@v2
  with:
    ref: ${{github.event.pull_request.head.ref}}
    repository: ${{github.event.pull_request.head.repo.full_name}}

Indeed, maybe it's worth doing this.

I don't think putting my personal license in the workflow publicly is the right way to go about either. As at any given moment it could be blocked because other people copied it over (which happens a lot as we know from experience).

[webbertakken]

However, "the changed workflow in merge commit" will be not tested, to prevent to run malicious workflows.
For example, if a PR is created that contains the following malicious workflow, it is safe until merged.

- run: |
    # Some malicious code with '$PASSWORD'
    ...
  env:
    PASSWORD: ${{ secrets.PASSWORD }}

There's another risk with running the main workflow pulling the pr code:

If any secret is passed into an action that action sets it as an env variable. Even if you use with: mapping it will use an environment variable INPUT_*. That means that if there is anything outside of the workflow that can access the env variables, it'd mean a leak.

There's very little that can shield against this, once this becomes possible.

I'm thinking this might not actually cause a problem in the end because we're running docker build or docker run with specific environment variables and the Dockerfile is not able to grab env variables from outside.

Any thoughts? It's important to get it right, because all repos should replicate similar behaviour to get rid of the public personal license.

cc: @GabLeRoux @mob-sakai @jonathanperret @davidmfinol @frostebite