[#1272] Actions: move deployment to Surge to a secure workflow

[dcshzj]

Resolves security issue.
Fixes #1272.

The current continuous integration workflow on GitHub Actions
includes a deployment to surge.sh. This is a security concern, as
it exposes the secrets configured in GitHub Actions to potential
bad actors when they create a pull request.

Let's separate the deployment to surge.sh as a different workflow
and pass the generated RepoSense report as well as the MarkBind
documentation website as an artifact. This reduces the exposure of
repository secrets to untrusted code and improves security, as the
untrusted code cannot be executed when repository secrets are made
available to the workflow.

Let's also change the way the deployment previews are reported in
the pull request by taking advantage of the deployment status
available in the GitHub API.

[fzdy1914]

LGTM

[fzdy1914]

@dcshzj Can you propose a nicer commit message that describes the dangering we are facing and how we are going to solve it? It is a good PR to practice your commit message writing ability.

[dcshzj]

@fzdy1914 I have updated the commit message, is it okay?

[fzdy1914]

The commit message is good now. But the sample run you provided does not have a showcase of how current deployment will be.

You have mentioned that workflow_run job can only execute if the file is on the default branch. What branch you are referring to? Is it master? It will be bad since we want to preview PRs to other branches also.

Why we have to use the workflow_run job? Why a pull_request event can not do the same thing?

[dcshzj]

The commit message is good now. But the sample run you provided does not have a showcase of how current deployment will be.

Sorry, are you referring to the deployment to surge? It's not included because only this repository has access to the secret, so only jobs that run on this repository will be able to successfully deploy to surge.

You have mentioned that workflow_run job can only execute if the file is on the default branch. What branch you are referring to? Is it master? It will be bad since we want to preview PRs to other branches also.

Yep it is only the master branch. This is a restriction noted in the GitHub documentation. However, this is referring to the workflow file being present in the master branch, but any PRs that is created to non-default branches will still work.

This is an example where it is a PR from one branch to another non-default branch: dcshzj#3

Why we have to use the workflow_run job? Why a pull_request event can not do the same thing?

The workflow_run job is needed because we can only run the deployment after the CI workflow has completed, since it is that workflow that generates the artifact. The pull_request event will run concurrently, causing it to try and get an artifact that has not been created yet.

[fzdy1914]

Sorry, are you referring to the deployment to surge? It's not included because only this repository has access to the secret, so only jobs that run on this repository will be able to successfully deploy to surge.

Can you set up a surge for your repo? This set up will not conflict with the setting of RepoSense.

Yep it is only the master branch. This is a restriction noted in the GitHub documentation. However, this is referring to the workflow file being present in the master branch, but any PRs that is created to non-default branches will still work.

OK, in this case. To have a test, can you temporarily merge your new branch in your master (and revert it when the testing is done)? And then, send one PR to master and another PR to another branch to see if the workflow goes well?

[fzdy1914]

If the workflow-run can not work. I found this and this links that can also solve our issue.

[dcshzj]

@fzdy1914 Here are the links for the workflows:

• PR to master branch workflow: https://github.com/dcshzj/RepoSense/runs/1721181834?check_suite_focus=true
• PR to non-default branch workflow: https://github.com/dcshzj/RepoSense/runs/1721189029?check_suite_focus=true

Seems to work as expected.

[fzdy1914]

Seems to work as expected.

Yep, it is working. However, to be more precise, can you refer to the way how deploy-surge.sh create a state in PR checklist to also create an entry for the surge workflow? So in case, we want to navigate to the detail of it, we do not need to find it in the action tab but directly in the checklist?

But you can argue that showing the PR preview links is enough to indicate that the workflow is running without any error. But anyway, I am thinking of effectively locating certain workflow if we indeed need to find it.

[Tejas2805]

@dcshzj Thank you for the quick work on the PR.

I think it would be a good practice to document this in some repository or in some way since we all won't be working on the project all the time. This would help future maintainers and developers be more prepared and avoid such code.

@damithc @fzdy1914 What do you think?

[damithc]

I think it would be a good practice to document this in some repository or in some way since we all won't be working on the project all the time. This would help future maintainers and developers be more prepared and avoid such code.

@damithc @fzdy1914 What do you think?

Sure, can add a note in the DG. Alternatively, a good commit message should be enough as it can be reached using git or github?

[dcshzj]

Yep, it is working. However, to be more precise, can you refer to the way how deploy-surge.sh create a state in PR checklist to also create an entry for the surge workflow? So in case, we want to navigate to the detail of it, we do not need to find it in the action tab but directly in the checklist?

I have implemented this by expanding on the commit status that we have used in the past. I also took the opportunity to remove the redundant code, since we are not doing surge.sh builds for pushes. Thus, the failure of the pull_request_target job is expected, due to changes in the way the script needs to be called.

GitHub Actions will now first report a pending status first. If any step fails for some reason, a failure status will be reported for the dashboard and docs deployments. Otherwise, it will remain pending until the deployment to surge is completed, then a success status will be reported.

A sample PR is available here: dcshzj#2

I think it would be a good practice to document this in some repository or in some way since we all won't be working on the project all the time. This would help future maintainers and developers be more prepared and avoid such code.

Would it be okay to keep it as part of the commit message? I think the separation of workflow files would raise eyebrows, but the commit history will have this PR for record.

[fzdy1914]

Set the env_url to pull_request event if fail, but to workflow even when success.

[dcshzj]

It should be working now. To verify:

• PR Test GitHub Actions for forked repositories #1402 tests if the GITHUB_TOKEN works as intended for forked repositories. This is verified by the existence of the "Pending" status for the deployment of the RepoSense report and documentation. The status is not at "Success" due to that step being run in workflow_run, which is not yet available in the default branch.
• PR Test dcshzj/RepoSense#2 tests if the actual deployment to surge.sh works. This is verified by the successful deployment for both the RepoSense report and documentation.

Untrusted code is now only run in integration.yml, which does not require secrets. Steps that require secrets are either in pending.yml or in surge.yml.

[fzdy1914]

LGTM