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: move from io/ioutil to io and os packages #353

Merged
merged 1 commit into from
Dec 18, 2021
Merged

refactor: move from io/ioutil to io and os packages #353

merged 1 commit into from
Dec 18, 2021

Conversation

Juneezee
Copy link
Contributor

@Juneezee Juneezee commented Nov 6, 2021

The io/ioutil package has been deprecated in Go 1.16 (See https://golang.org/doc/go1.16#ioutil). Since age has been upgraded to Go 1.17 in 36b0a4f, this PR replaces the existing io/ioutil functions with their new definitions in io and os packages.

@mikecook
Copy link
Contributor

Rerun build step, it appears Filo fixed the Github secret for osslsigncode certkey pass that failed this build.

@Juneezee
Copy link
Contributor Author

Rerun build step, it appears Filo fixed the Github secret for osslsigncode certkey pass that failed this build.

@mikecook The build step is still failing. I don't think it has been fixed yet.

@mikecook
Copy link
Contributor

mikecook commented Dec 18, 2021

Whoops, his last build that passed wasn't using his windows build commit.
11 days ago the build passed for Filo with windows in the build https://github.com/FiloSottile/age/runs/4446552982?check_suite_focus=true#step:4:72 I'm not sure what's going on.

@FiloSottile 's windows build step broke it here 3bd9ab8#diff-5c3fa597431eda03ac3339ae6bf7f05e1a50d6fc7333679ec38e21b337cb6721R40

@FiloSottile can you check secrets.SIGN_PASS is correct?

@mikecook
Copy link
Contributor

mikecook commented Dec 18, 2021

Workflows that use environments are tricky around secrets because of the potential for secret leakage

For a private repo you can enable this setting:
Send secrets to workflows from fork pull requests.
https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/

But for a public repo the secrets aren't shared:
https://docs.github.com/en/actions/learn-github-actions/events-that-trigger-workflows#pull-request-events-for-forked-repositories
With the exception of GITHUB_TOKEN, secrets are not passed to the runner when a workflow is triggered from a forked repository.

https://docs.github.com/en/actions/security-guides/encrypted-secrets#about-encrypted-secrets
For secrets stored at the environment level, you can enable required reviewers to control access to the secrets. A workflow job cannot access environment secrets until approval is granted by required approvers.

https://docs.github.com/en/actions/managing-workflow-runs/reviewing-deployments
To approve the job, click Approve and deploy. Once a job is approved (and any other environment protection rules have passed), the job will proceed. At this point, the job can access any secrets stored in the environment.

Options:

  1. Move secrets.SIGN_PASS from the environment level to the repository level (BAD: could leak exe signing key pass)
  2. Set Required Reviewers and have Reviewer run the job which would allow job access to secrets (Extra work for Reviewers)
  3. Leave the building for Releases by removing https://github.com/FiloSottile/age/blob/main/.github/workflows/build.yml#L6 (and unless the github builds are used for testing, probably also remove the on.push above on.pull_request.)
  4. Split build and exe signing into separate workflow steps to allow builds for forked PRs.

mikecook added a commit to mikecook/age that referenced this pull request Dec 18, 2021
Environmental secrets are not shared to workflows run from forks without extra work by Maintainers. 

The existing build workflow uses a secret to sign windows exe's and is breaking for all external PRs. 

Builds are also probably unnecessary for PRs.

See: FiloSottile#353 (comment)
@mikecook
Copy link
Contributor

rebase on main and it should work now

The io/ioutil package has been deprecated as of Go 1.16, see
https://golang.org/doc/go1.16#ioutil. This commit replaces the existing
io/ioutil functions with their new definitions in io and os packages.

Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
@Juneezee
Copy link
Contributor Author

@mikecook Great, the build step now passes.

@FiloSottile
Copy link
Owner

The version in the go.mod is not really the minimum supported version, but I don't feel the need to support versions of Go that are out of security support, so we can count on Go 1.16+.

@FiloSottile FiloSottile merged commit 7665b87 into FiloSottile:main Dec 18, 2021
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