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

build: add automated releases and pre-commit hooks #289

Closed
wants to merge 18 commits into from

Conversation

Maytha8
Copy link

@Maytha8 Maytha8 commented May 14, 2023

What is the purpose of this pull request? (put an "X" next to item)

  • Documentation update
  • Bug fix
  • New feature
  • Other, please explain:

What changes did you make? (Give an overview)

  • Install and configure semantic releases.
  • Modify the GitHub Workflow to run semantic-release.
  • Add commit hooks that run commitlint, standard, and ts-standard.

Which issue (if any) does this pull request address?

Is there anything you'd like reviewers to focus on?

Please check that all the configuration is what is needed.
Before merging, make sure the GitHub repository secrets GH_TOKEN and NPM_TOKEN are set to a GitHub Personal Access Token and NPM Access Token respectively.

@welcome
Copy link

welcome bot commented May 14, 2023

🙌 Thanks for opening this pull request! You're awesome.

@Maytha8 Maytha8 marked this pull request as ready for review May 14, 2023 06:37
@socket-security
Copy link

socket-security bot commented May 14, 2023

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Issue Package Version Note Source
Network access http-proxy-agent 7.0.0 package.json via @semantic-release/[email protected], @semantic-release/[email protected], @semantic-release/[email protected], @semantic-release/[email protected], [email protected]
Network access https-proxy-agent 7.0.0 package.json via @semantic-release/[email protected], @semantic-release/[email protected], @semantic-release/[email protected], @semantic-release/[email protected], [email protected]
Network access npm 9.7.1 package.json via @semantic-release/[email protected], @semantic-release/[email protected], @semantic-release/[email protected], @semantic-release/[email protected], [email protected]
Network access @octokit/request 6.2.5 package.json via @semantic-release/[email protected], @semantic-release/[email protected], @semantic-release/[email protected], @semantic-release/[email protected], [email protected]
Network access node-fetch 2.6.11 package.json via @semantic-release/[email protected], @semantic-release/[email protected], @semantic-release/[email protected], @semantic-release/[email protected], [email protected]
Network access semantic-release 21.0.5 package.json via @semantic-release/[email protected], @semantic-release/[email protected], @semantic-release/[email protected], @semantic-release/[email protected]
Network access agent-base 7.1.0 package.json via @semantic-release/[email protected], @semantic-release/[email protected], @semantic-release/[email protected], @semantic-release/[email protected], [email protected]
Network access @semantic-release/github 8.1.0 package.json
Network access @semantic-release/github 9.0.3 package.json via @semantic-release/[email protected], @semantic-release/[email protected], @semantic-release/[email protected], @semantic-release/[email protected], [email protected]
Network access config-chain 1.1.13 package.json via @semantic-release/[email protected], @semantic-release/[email protected], @semantic-release/[email protected], @semantic-release/[email protected], [email protected]

Next steps

What is network access?

This module accesses the network.

Packages should remove all network access that isn't functionally unnecessary. Consumers should audit network access to ensure legitimate use.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore [email protected] bar@* or ignore all packages with @SocketSecurity ignore-all

Copy link
Member

@theoludwig theoludwig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution, we appreciate it!

I left some comments to open the discussion. 😄

.commitlintrc.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated
@@ -1,7 +1,7 @@
{
"name": "ts-standard",
"description": "TypeScript Standard Style based on StandardJS",
"version": "12.0.2",
"version": "12.0.3",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we automate it, I think we should not increase the version ourselves.
I think we should use 0.0.0-development instead.

https://github.com/semantic-release/semantic-release/blob/18bd0c490483008230c57ae6a73d07a09dd00d0d/package.json#L4

Copy link
Author

@Maytha8 Maytha8 Jun 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

semantic-release will bump the version for you, and @semantic-release/git will commit and push the version bump to the git repo (This is why the accidental version bump seen here happened, as I was testing semantic-release). The version won't have to be increased manually. 😄

You can see this in action in a test repo I made; here's an example commit of what it will do:
https://github.com/Maytha8/ts-standard-test/commit/b00d5b3fefff87dac6bf0c4f0dccf3c11d116156#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519
Here you can see that it knew what the next version was and bumped it.

The only reason semantic-release have their own version set to 0.0.0-development is because they don't use @semantic-release/npm on their own project.

If you'd like this behavior of leaving the version at 0.0.0-development, then we could remove package.json from the assets property of @semantic-release/git in .releaserc.yml, thus not committing the package.json version bump.

Comment on lines +42 to +44
- assets:
- CHANGELOG.md
- package.json
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That might be best to not have any assets.
The changelog will be published with the GitHub Release, what do you think? 😄

Copy link
Author

@Maytha8 Maytha8 Jun 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHANGELOG.md
This file already exists in this repo, so I might as well setup semantic-release to update it. 😃
@semantic-release/changelog will update the changelog automatically for us.

package.json
@semantic-release/npm updates the package.json version, so might as well commit that.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #289 (comment) regarding package.json.

- run: 'npm run lint:ts-standard'
- run: 'npm run test'
- run: npm install
- run: npm run lint:commit -- --from "7b4219a67b4fe94b1a1487b7d545d0cc7742f770" --to "${{ github.sha }}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why adding this from flag with a hard-coded commit hash?

Copy link
Author

@Maytha8 Maytha8 Jun 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because some old commits in the commit history don't follow the format and cause commitlint to error, as you can see below. (This log excludes the errors caused by dependabot's messages being too long, as body-max-line-len was disabled at the time of running this.)

$ npm run lint:commit -- --to "79b63eb"

> [email protected] lint:commit
> commitlint --to 79b63eb

⧗   input: feat(package): Allow multiple projects to be used (#163)
✖   subject must not be sentence-case, start-case, pascal-case, upper-case [subject-case]

✖   found 1 problems, 0 warnings
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

⧗   input: fix(badges): Fix readme file badges
✖   subject must not be sentence-case, start-case, pascal-case, upper-case [subject-case]

✖   found 1 problems, 0 warnings
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

⧗   input: fix(typescript): Try to make typescript a peerDependency
✖   subject must not be sentence-case, start-case, pascal-case, upper-case [subject-case]

✖   found 1 problems, 0 warnings
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

⧗   input: Initial commit
✖   subject may not be empty [subject-empty]
✖   type may not be empty [type-empty]

✖   found 2 problems, 0 warnings
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

I've hardcoded it from the latest commit on standard/ts-standard as that's the last commit before my commits, and this is when I've setup a precommit hook for commitlint.

.github/workflows/nodejs.yml Outdated Show resolved Hide resolved
@socket-security
Copy link

New and updated dependency changes detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives1 Size Publisher
husky 🆕 8.0.3 filesystem, shell, environment +0 6.44 kB typicode
@semantic-release/changelog 🆕 6.0.3 filesystem +159 27.4 MB semantic-release-bot
@semantic-release/git 🆕 10.0.1 None +160 27.4 MB semantic-release-bot
@semantic-release/npm 🆕 10.0.4 filesystem +155 27.4 MB semantic-release-bot
semantic-release 🆕 21.0.5 network, environment +155 27.4 MB semantic-release-bot
@semantic-release/github 🆕 8.1.0 network, filesystem +165 27.8 MB semantic-release-bot

Footnotes

  1. https://docs.socket.dev

@Maytha8
Copy link
Author

Maytha8 commented Jun 13, 2023

@theoludwig What are your thoughts on:

  • configuring the workflow to publish to a (new) dev tag on npm, set the version to dev-<timestamp>, and create a pre-release on GitHub; and
  • adding a release script to package.json that allows you to easily publish to the latest tag and create a distribution release.

Edit: the only issues with this are that manually releasing can be a security vulnerability and npm provenance doesn't apply here (consequences of publishing outside of a CI environment).

commitlint's --from flag has anyways been hardcoded to ignore old
dependabot messages, so this rule should be re-enabled.
This is to ensure only commits pushed to the repo are published, and not
pull requests that have not yet been merged (we only want tests to run
    for these).
- run: npm install
- run: npx semantic-release
env:
GITHUB_TOKEN: ${{ secrets.GH_TOKEN }}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@theoludwig what are your thoughts on using a Personal Access Token instead of the default GITHUB_TOKEN secret that GitHub provides?

From a note in semantic-release's documentation:

Automatically populated GITHUB_TOKEN cannot be used if branch protection is enabled for the target branch. It is not advised to mitigate this limitation by overriding an automatically populated GITHUB_TOKEN variable with a Personal Access Token, as it poses a security risk. Since Secret Variables are available for Workflows triggered by any branch, it becomes a potential vector of attack, where a Workflow triggered from a non-protected branch can expose and use a token with elevated permissions, yielding branch protection insignificant. One can use Personal Access Tokens in trusted environments, where all developers should have the ability to perform administrative actions in the given repository and branch protection is enabled solely for convenience purposes, to remind about required reviews or CI checks.

I'm unsure to whether ts-standard has branch protection enabled on the master branch (I'm guessing yes).

@Maytha8 Maytha8 closed this Oct 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Automated release process
2 participants