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
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ jobs:
with:
node-version: ${{ matrix.node-version }}

- name: Update npm
run: npm i -g [email protected]

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
Original file line number Diff line number Diff line change
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 [email protected]
- 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/[email protected]
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"scripts": {
"test": "../node_modules/.bin/webpack && node build/main.js"
"test": "webpack && node build/main.js"
}
}