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

Use SLSA publish action to include verified build information #211

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
3 changes: 3 additions & 0 deletions .github/workflows/CI.yml
Expand Up @@ -18,6 +18,9 @@ jobs:
with:
node-version: ${{ matrix.node-version }}

- name: Update npm
run: npm i -g npm@8.19.4

weswigham marked this conversation as resolved.
Show resolved Hide resolved
- name: Run tests
run: node ./test/runTests.js

Expand Down
46 changes: 37 additions & 9 deletions .github/workflows/publish.yaml
Expand Up @@ -5,16 +5,44 @@ on:
types: [created]

jobs:
publish-npm:
test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1
- name: Use node version 18
uses: actions/setup-node@v1
with:
node-version: 14
registry-url: https://registry.npmjs.org/
- run: npm i
- run: node test/validateModuleExportsMatchCommonJS/index.js
- run: npm publish
env:
NODE_AUTH_TOKEN: ${{secrets.npm_token}}
node-version: 18
- name: Update npm
run: npm i -g npm@8.19.4
- name: Run tests
run: npm run test
build:
needs: [test]
permissions:
id-token: write # For signing
contents: read # For repo checkout.
actions: read # For getting workflow run info.
uses: slsa-framework/slsa-github-generator/.github/workflows/builder_nodejs_slsa3.yml@v1.7.0
Copy link

Choose a reason for hiding this comment

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

v1.8.0 fixes some issues with unscoped packages.

Suggested change
uses: slsa-framework/slsa-github-generator/.github/workflows/builder_nodejs_slsa3.yml@v1.7.0
uses: slsa-framework/slsa-github-generator/.github/workflows/builder_nodejs_slsa3.yml@v1.8.0

publish:
needs: [build]
runs-on: ubuntu-latest
steps:
- name: Set up Node registry authentication
uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0
with:
node-version: 18
registry-url: "https://registry.npmjs.org"

- name: publish
id: publish
uses: slsa-framework/slsa-github-generator/actions/nodejs/publish@e55b76ce421082dfa4b34a6ac3c5e59de0f3bb58 # v1.7.0
with:
access: public
node-auth-token: ${{ secrets.npm_token }}
package-name: ${{ needs.build.outputs.package-name }}
package-download-name: ${{ needs.build.outputs.package-download-name }}
package-download-sha256: ${{ needs.build.outputs.package-download-sha256 }}
provenance-name: ${{ needs.build.outputs.provenance-name }}
provenance-download-name: ${{ needs.build.outputs.provenance-download-name }}
provenance-download-sha256: ${{ needs.build.outputs.provenance-download-sha256 }}
3 changes: 3 additions & 0 deletions package.json
Expand Up @@ -43,5 +43,8 @@
},
"./*": "./*",
"./": "./"
},
"scripts": {
"test": "node ./test/runTests.js && node test/validateModuleExportsMatchCommonJS/index.js"
Copy link
Member Author

Choose a reason for hiding this comment

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

Previously the publish skipped the runTests.js part, but I don't see the harm in including it - it's fast.

weswigham marked this conversation as resolved.
Show resolved Hide resolved
}
}
2 changes: 1 addition & 1 deletion test/rollup-modules/package.json
@@ -1,5 +1,5 @@
{
"scripts": {
"test": "../node_modules/.bin/rollup -c rollup.config.js && node build/index.js"
"test": "rollup -c rollup.config.js && node build/index.js"
}
}
15 changes: 11 additions & 4 deletions test/runTests.js
Expand Up @@ -14,7 +14,14 @@ const tests = filesInTest
// Support setting up the test node modules
if (!filesInTest.includes("node_modules")) {
console.log("Installing Deps...");
spawnSync("npm", ["install"], { cwd: __dirname });
const res = spawnSync("npm", ["install"], { cwd: __dirname, shell: true });
Copy link
Member Author

Choose a reason for hiding this comment

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

At least on my machine, you can't actually find npm unless the shell: true command is passed so this actually checks the system PATH for npm (and, without the extra logging below, this failure was completely silent).

if (res.error) {
console.error(res.error);
process.exit(res.error.errno || -1);
}
if (res.output) {
console.log(res.output.toString());
}
console.log("Installed");
}

Expand All @@ -37,13 +44,13 @@ for (const test of tests) {
if (pgkJSON.dependencies || pgkJSON.devDependencies) {
const nodeModsInstalled = fs.existsSync(path.join(__dirname, test, "node_modules"));
if (!nodeModsInstalled) {
spawnSync("npm", ["install"], { cwd: path.join(__dirname, test) });
spawnSync("npm", ["install"], { cwd: path.join(__dirname, test), shell: true });
}
}

// Run the test command
const results = spawnSync("npm", ["test"], { cwd: path.join(__dirname, test) });
console.log(results.stdout.toString())
const results = spawnSync("npm", ["test"], { cwd: path.join(__dirname, test), shell: true });
console.log((results.stdout || "").toString())
if (results.status) {
console.log(chalk.bold.red("Error running test: ") + chalk.bold(test))
console.log(results.stderr.toString())
Expand Down
2 changes: 1 addition & 1 deletion test/snowpack-modules/package.json
Expand Up @@ -2,7 +2,7 @@
"type": "module",

"scripts": {
"test": "../node_modules/.bin/snowpack build; node build/index.js"
"test": "snowpack build && node build/index.js"
},
"engines": {
"node": "14"
Expand Down
2 changes: 1 addition & 1 deletion test/vite/package.json
@@ -1,5 +1,5 @@
{
"scripts": {
"test": "../node_modules/.bin/vite build && node build/index.js"
"test": "vite build && node build/index.js"
}
}
2 changes: 1 addition & 1 deletion test/webpack-4-modules/package.json
@@ -1,5 +1,5 @@
{
"scripts": {
"test": "../node_modules/.bin/webpack && node build/main.js"
"test": "webpack && node build/main.js"
}
}