From a94f0ac4502be93daec294f8bc54ca6aa27345a4 Mon Sep 17 00:00:00 2001 From: ssvoss Date: Sat, 7 Nov 2026 21:45:24 -0500 Subject: [PATCH 1/4] CORE-6263: say why App CI audits npm dependencies itself This repository has only devDependencies, and run_npm_audit audits production dependencies only (--omit=dev), so calling the reusable workflow would audit nothing. The comment keeps someone from swapping the App CI audit step for run_npm_audit. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/internal_on_push_ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/internal_on_push_ci.yml b/.github/workflows/internal_on_push_ci.yml index f21c8d1..43cdc3f 100644 --- a/.github/workflows/internal_on_push_ci.yml +++ b/.github/workflows/internal_on_push_ci.yml @@ -54,6 +54,8 @@ jobs: id: install run: npm ci + # The audit runs here, not through run_npm_audit: this repository has only devDependencies, + # and run_npm_audit audits production dependencies only (--omit=dev), so it would check nothing. - name: Audit dependencies if: ${{ !cancelled() && steps.install.outcome == 'success' }} run: npm run audit:npm From 6f98412fd711211714631e1096cbe3407746ceaf Mon Sep 17 00:00:00 2001 From: ssvoss Date: Sat, 7 Nov 2026 21:45:24 -0500 Subject: [PATCH 2/4] CORE-6263: fix npm setup, setup-failure cells and baselined counts The first live run of run_repo_checks (core-github-actions#167, run 42438765570) found three defects: - Markdown and JS ran npm ci with the npm bundled with .nvmrc's Node (11.19.1 for 24.22.2), so a repository with engines.npm >=12.1.0 and engine-strict failed with EBADENGINE. Both workflows now install the npm version from package.json's packageManager (npm@X.Y.Z) first, and keep the bundled npm when there is none. - The Summary showed "skipped" for steps skipped because npm ci or mise failed, so a failed job's row had no failure in it. A step skipped in a failed job with no failing step now shows "setup failed"; a step skipped after a failing one, such as Validate after Init, still shows "skipped". - The baselined counts grepped for directive names anywhere, so READMEs, CHANGELOGs and test fixtures counted (markdown 3, js 6, gha 13 here), and git grep -c counted lines. The counts now match each directive's comment syntax and count occurrences: markdown 0, js 0, gha 4 (the two existing ignores plus the new npm step's two), shell 2. The run_repo_checks README now also says a skipped technology's check is named "Repo Checks / ", with no "/ Checks". Releases run_markdown_checks and run_js_checks 1.0.2, run_gha_checks, run_shell_checks and run_terraform_checks 1.0.3, and run_repo_checks 1.0.3, which calls them through ./. Co-Authored-By: Claude Opus 5.5 --- .../workflows/CHANGELOGS/run_gha_checks.md | 8 +++ .github/workflows/CHANGELOGS/run_js_checks.md | 11 ++++ .../CHANGELOGS/run_markdown_checks.md | 11 ++++ .../workflows/CHANGELOGS/run_repo_checks.md | 11 ++++ .../workflows/CHANGELOGS/run_shell_checks.md | 8 +++ .../CHANGELOGS/run_terraform_checks.md | 8 +++ .github/workflows/READMES/run_gha_checks.md | 12 ++-- .github/workflows/READMES/run_js_checks.md | 19 +++--- .../workflows/READMES/run_markdown_checks.md | 17 +++--- .github/workflows/READMES/run_repo_checks.md | 40 ++++++++---- .github/workflows/READMES/run_shell_checks.md | 4 +- .../workflows/READMES/run_terraform_checks.md | 18 +++--- .../TESTS/run_gha_checks.unit.test.js | 23 ++++--- .../TESTS/run_js_checks.unit.test.js | 27 +++++--- .../TESTS/run_markdown_checks.unit.test.js | 24 ++++++-- .../TESTS/run_repo_checks.unit.test.js | 31 ++++++++++ .../TESTS/run_shell_checks.unit.test.js | 18 ++++-- .../TESTS/run_terraform_checks.unit.test.js | 5 +- .../workflows/TESTS/workflow-test-helpers.js | 61 ++++++++++++++++++- .github/workflows/run_gha_checks.yml | 4 +- .github/workflows/run_js_checks.yml | 17 +++++- .github/workflows/run_markdown_checks.yml | 17 +++++- .github/workflows/run_repo_checks.yml | 17 ++++-- .github/workflows/run_shell_checks.yml | 4 +- .github/workflows/run_terraform_checks.yml | 2 +- 25 files changed, 329 insertions(+), 88 deletions(-) diff --git a/.github/workflows/CHANGELOGS/run_gha_checks.md b/.github/workflows/CHANGELOGS/run_gha_checks.md index 6153942..2a1de84 100644 --- a/.github/workflows/CHANGELOGS/run_gha_checks.md +++ b/.github/workflows/CHANGELOGS/run_gha_checks.md @@ -2,6 +2,14 @@ All notable changes to the **run_gha_checks** reusable workflow are documented in this file. +## 1.0.3 + +### Fixed + +- The `baselined` output counts only real inline directives (`# zizmor: ignore[...]` in YAML files + under `.github`), and counts each one rather than each line. Names in documentation, changelogs + and test fixtures inflated the count. + ## 1.0.2 ### Changed diff --git a/.github/workflows/CHANGELOGS/run_js_checks.md b/.github/workflows/CHANGELOGS/run_js_checks.md index 0df4631..e0e4f2e 100644 --- a/.github/workflows/CHANGELOGS/run_js_checks.md +++ b/.github/workflows/CHANGELOGS/run_js_checks.md @@ -2,6 +2,17 @@ All notable changes to the **run_js_checks** reusable workflow are documented in this file. +## 1.0.2 + +### Fixed + +- Installs the npm version from `package.json`'s `packageManager` (`npm@X.Y.Z`) before `npm ci`, and + keeps the npm that ships with `.nvmrc`'s Node when there is none. `npm ci` failed with EBADENGINE in + repositories whose `engines.npm` needs a newer npm than Node bundles and that set `engine-strict`. +- The `baselined` output counts only real inline directives (`//` or `/*` `eslint-disable…` and + `prettier-ignore` comments in JS and TS files), and counts each one rather than each line. Names + in documentation, changelogs and test fixtures inflated the count. + ## 1.0.1 ### Changed diff --git a/.github/workflows/CHANGELOGS/run_markdown_checks.md b/.github/workflows/CHANGELOGS/run_markdown_checks.md index da03245..d63ed95 100644 --- a/.github/workflows/CHANGELOGS/run_markdown_checks.md +++ b/.github/workflows/CHANGELOGS/run_markdown_checks.md @@ -2,6 +2,17 @@ All notable changes to the **run_markdown_checks** reusable workflow are documented in this file. +## 1.0.2 + +### Fixed + +- Installs the npm version from `package.json`'s `packageManager` (`npm@X.Y.Z`) before `npm ci`, and + keeps the npm that ships with `.nvmrc`'s Node when there is none. `npm ci` failed with EBADENGINE in + repositories whose `engines.npm` needs a newer npm than Node bundles and that set `engine-strict`. +- The `baselined` output counts only real inline directives (`\ntext\n\n', - 'docs/guide.md': '\n', + 'docs/guide.md': + '\n
\n', + }); + + expect(result.status).toBe(0); + expect(result.outputs.count).toBe('5'); + }); + + test("doesn't count the directive name in prose, inline code, other files or .core-services", () => { + const result = count({ + 'README.md': 'Use `` sparingly; markdownlint-disable is baselined.\n', + 'CHANGELOG.md': '- Counts inline `markdownlint-disable` comments.\n', '.core-services/standard.md': '\n', - 'notes.txt': 'markdownlint-disable is not counted outside Markdown\n', + 'notes.txt': '\n', + 'test.js': "const fixture = '';\n", [`.github/workflows/${workflowFile}`]: readWorkflow(workflowFile), }); expect(result.status).toBe(0); - expect(result.outputs.count).toBe('3'); + expect(result.outputs.count).toBe('0'); }); }); diff --git a/.github/workflows/TESTS/run_repo_checks.unit.test.js b/.github/workflows/TESTS/run_repo_checks.unit.test.js index 2951e24..d888d98 100644 --- a/.github/workflows/TESTS/run_repo_checks.unit.test.js +++ b/.github/workflows/TESTS/run_repo_checks.unit.test.js @@ -165,6 +165,37 @@ describe('run_repo_checks Summary', () => { expect(result.summary).toContain('**Standard:** ❌ fail'); }); + test('shows a setup failure when a failed job skipped steps without a failing one', () => { + const result = summarize({ + detect: detected({ 'markdown-files': '74', 'js-files': '27', 'terraform-files': '3' }), + gha: skipped, + terraform: { + result: 'failure', + outputs: { + init: 'skipped', + validate: 'skipped', + lint: 'skipped', + format: 'skipped', + scan: 'skipped', + baselined: '0', + }, + }, + markdown: { result: 'failure', outputs: { lint: 'skipped', baselined: '0' } }, + shell: skipped, + js: { result: 'failure', outputs: { lint: 'skipped', format: 'skipped', baselined: '0' } }, + standard: { result: 'success', outputs: { result: 'success' } }, + }); + + expect(result.status).toBe(0); + expect(result.summary).toContain( + '| Terraform | 3 | ❌ setup failed | ❌ setup failed | ❌ setup failed | ❌ setup failed | ❌ setup failed | 0 |' + ); + expect(result.summary).toContain('| Markdown | 74 | — | — | ❌ setup failed | — | — | 0 |'); + expect(result.summary).toContain( + '| JavaScript/TypeScript | 27 | — | — | ❌ setup failed | ❌ setup failed | — | 0 |' + ); + }); + test('marks every technology unknown when Detect failed', () => { const result = summarize({ detect: { result: 'failure', outputs: {} }, diff --git a/.github/workflows/TESTS/run_shell_checks.unit.test.js b/.github/workflows/TESTS/run_shell_checks.unit.test.js index 74aaae3..ba74341 100644 --- a/.github/workflows/TESTS/run_shell_checks.unit.test.js +++ b/.github/workflows/TESTS/run_shell_checks.unit.test.js @@ -6,19 +6,25 @@ describe('run_shell_checks baselined count', () => { const count = files => runStepInRepository(workflowFile, 'checks', 'Count baselined issues', files); - test('counts inline shellcheck disables in any tracked file outside .core-services', () => { + test('counts each inline shellcheck disable directive in scripts and workflow run blocks', () => { const result = count({ 'scripts/deploy.sh': '#!/bin/bash\n# shellcheck disable=SC2086\necho $1\n', - 'scripts/tool': '#!/usr/bin/env bash\n# shellcheck disable=SC2034,SC2154\n', - '.core-services/generated/hook.sh': '# shellcheck disable=SC1091\n', + 'scripts/tool': '#!/usr/bin/env bash\n#shellcheck disable=SC2034,SC2154\n', + '.github/workflows/ci.yml': ' run: |\n # shellcheck disable=SC2016\n', }); expect(result.status).toBe(0); - expect(result.outputs.count).toBe('2'); + expect(result.outputs.count).toBe('3'); }); - test("doesn't count its own pattern", () => { - const result = count({ [`.github/workflows/${workflowFile}`]: readWorkflow(workflowFile) }); + test("doesn't count the directive in docs, strings, .core-services or its own pattern", () => { + const result = count({ + 'README.md': '# shellcheck disable=SC2086\n', + 'CHANGELOG.md': '- Counts inline `# shellcheck disable=` directives.\n', + 'test/a.test.js': "const script = '#!/bin/bash\\n# shellcheck disable=SC2086\\n';\n", + '.core-services/generated/hook.sh': '# shellcheck disable=SC1091\n', + [`.github/workflows/${workflowFile}`]: readWorkflow(workflowFile), + }); expect(result.status).toBe(0); expect(result.outputs.count).toBe('0'); diff --git a/.github/workflows/TESTS/run_terraform_checks.unit.test.js b/.github/workflows/TESTS/run_terraform_checks.unit.test.js index 9d5aeb9..fc0010d 100644 --- a/.github/workflows/TESTS/run_terraform_checks.unit.test.js +++ b/.github/workflows/TESTS/run_terraform_checks.unit.test.js @@ -18,12 +18,13 @@ describe('run_terraform_checks baselined count', () => { 'terraform/.checkov.baseline': baseline(['CKV_AWS_18', 'CKV_AWS_144']), 'terraform/modules/x/.checkov.baseline': baseline(['CKV_AWS_21']), 'terraform/main.tf': - 'resource "aws_s3_bucket" "a" {\n #checkov:skip=CKV_AWS_19: reason\n}\n# tflint-ignore: terraform_unused_declarations\n', + 'resource "aws_s3_bucket" "a" {\n #checkov:skip=CKV_AWS_19: reason\n # checkov:skip=CKV_AWS_20: reason\n}\n# tflint-ignore: terraform_unused_declarations\n// tflint-ignore-file: terraform_naming_convention\n', '.core-services/vendored.tf': '#checkov:skip=CKV_AWS_1: vendored\n', + 'README.md': '#checkov:skip=CKV_AWS_19: documented\n', }); expect(result.status).toBe(0); - expect(result.outputs.count).toBe('5'); + expect(result.outputs.count).toBe('7'); }); test("reports zero without a baseline or inline ignores, and doesn't count its own patterns", () => { diff --git a/.github/workflows/TESTS/workflow-test-helpers.js b/.github/workflows/TESTS/workflow-test-helpers.js index b2daa0d..d14b8b8 100644 --- a/.github/workflows/TESTS/workflow-test-helpers.js +++ b/.github/workflows/TESTS/workflow-test-helpers.js @@ -13,7 +13,10 @@ function extractRunScript(workflowFile, jobId, stepName) { const lines = readWorkflow(workflowFile).split('\n'); const jobStart = lines.indexOf(` ${jobId}:`); const stepStart = jobStart === -1 ? -1 : lines.indexOf(` - name: ${stepName}`, jobStart); - const runStart = stepStart === -1 ? -1 : lines.indexOf(' run: |', stepStart); + const runStart = + stepStart === -1 + ? -1 + : lines.findIndex((line, index) => index > stepStart && /^ {8}run: \|( #.*)?$/.test(line)); if (jobStart === -1 || stepStart === -1 || runStart === -1) { throw new Error( @@ -97,8 +100,64 @@ function runStepInRepository(workflowFile, jobId, stepName, files, env) { } } +function runSetUpNpm(workflowFile, packageJson) { + const bin = fs.mkdtempSync(path.join(os.tmpdir(), 'repo-checks-npm-')); + const log = path.join(bin, 'calls'); + fs.writeFileSync( + path.join(bin, 'npm'), + `#!/usr/bin/env bash\nif [[ "$1" == --version ]]; then echo 11.19.1; else echo "$*" >> '${log}'; fi\n`, + { mode: 0o755 } + ); + try { + const result = runStepInRepository( + workflowFile, + 'checks', + 'Set up npm', + { 'package.json': JSON.stringify(packageJson) }, + { PATH: `${bin}:${process.env.PATH}` } + ); + return { ...result, calls: fs.existsSync(log) ? fs.readFileSync(log, 'utf8') : '' }; + } finally { + fs.rmSync(bin, { recursive: true, force: true }); + } +} + +function describeSetUpNpm(workflowFile) { + describe(`${workflowFile} Set up npm`, () => { + test('installs the npm version from packageManager', () => { + const result = runSetUpNpm(workflowFile, { packageManager: 'npm@12.2.1' }); + + expect(result.status).toBe(0); + expect(result.calls).toBe('install --global npm@12.2.1\n'); + }); + + test('drops a packageManager hash', () => { + const result = runSetUpNpm(workflowFile, { packageManager: 'npm@12.2.1+sha512.abc123' }); + + expect(result.status).toBe(0); + expect(result.calls).toBe('install --global npm@12.2.1\n'); + }); + + test.each([ + ['no packageManager', {}], + ['another package manager', { packageManager: 'pnpm@10.0.0' }], + ['a range instead of a version', { packageManager: 'npm@^12' }], + ['extra text after the version', { packageManager: 'npm@12.2.1; echo hi' }], + ['a non-string packageManager', { packageManager: { name: 'npm' } }], + ])('keeps the bundled npm with %s', (_, packageJson) => { + const result = runSetUpNpm(workflowFile, packageJson); + + expect(result.status).toBe(0); + expect(result.calls).toBe(''); + expect(result.stdout).toContain('using the bundled npm'); + expect(result.stdout).toContain('npm 11.19.1'); + }); + }); +} + module.exports = { createRepository, + describeSetUpNpm, extractRunScript, readWorkflow, runScript, diff --git a/.github/workflows/run_gha_checks.yml b/.github/workflows/run_gha_checks.yml index e4a2e22..5fdb631 100644 --- a/.github/workflows/run_gha_checks.yml +++ b/.github/workflows/run_gha_checks.yml @@ -16,7 +16,7 @@ on: description: Outcome of the scan step (success, failure, cancelled or skipped) value: ${{ jobs.checks.outputs.scan }} baselined: - description: Number of inline zizmor ignore comments under .github + description: Number of inline zizmor ignore comments in YAML files under .github value: ${{ jobs.checks.outputs.baselined }} permissions: {} @@ -64,6 +64,6 @@ jobs: id: baseline if: ${{ !cancelled() }} run: | - inline=$({ git grep -c 'zizmor[:] *ignore' -- .github || true; } | awk -F: '{ sum += $NF } END { print sum + 0 }') + inline=$({ git grep -ohE '(^|[[:space:]])#[[:space:]]*zizmor:[[:space:]]*ignore\[' -- '.github/*.yml' '.github/*.yaml' || true; } | awk 'END { print NR }') echo "Inline zizmor ignores: $inline" echo "count=$inline" >> "$GITHUB_OUTPUT" diff --git a/.github/workflows/run_js_checks.yml b/.github/workflows/run_js_checks.yml index 01ba127..c2f103c 100644 --- a/.github/workflows/run_js_checks.yml +++ b/.github/workflows/run_js_checks.yml @@ -20,7 +20,7 @@ on: description: Outcome of the format-check step (success, failure, cancelled or skipped) value: ${{ jobs.checks.outputs.format }} baselined: - description: Number of inline ESLint disable and Prettier ignore comments + description: Number of inline ESLint disable and Prettier ignore comments in JS and TS files value: ${{ jobs.checks.outputs.baselined }} permissions: {} @@ -55,6 +55,19 @@ jobs: registry-url: https://npm.pkg.github.com scope: '@opensesame' + - name: Set up npm + id: npm + run: | # zizmor: ignore[adhoc-packages] Installs the npm version the repository pins in packageManager; setup-node has no npm version input. + package_manager=$(jq -r 'if (.packageManager | type) == "string" then .packageManager else "" end' package.json) + if [[ "$package_manager" =~ ^npm@([0-9]+\.[0-9]+\.[0-9]+)(\+.*)?$ ]]; then + version=${BASH_REMATCH[1]} + echo "Installing npm $version from packageManager" + npm install --global "npm@$version" + else + echo "No npm@X.Y.Z packageManager; using the bundled npm" + fi + echo "npm $(npm --version)" + - name: Install dependencies id: install env: @@ -75,6 +88,6 @@ jobs: id: baseline if: ${{ !cancelled() }} run: | - inline=$({ git grep -c -E 'eslint-disabl[e]|prettier-igno[r]e' -- ':!.core-services' || true; } | awk -F: '{ sum += $NF } END { print sum + 0 }') + inline=$({ git grep -ohE '(^|[[:space:]{])(//|/\*)[[:space:]]*(eslint-disable|prettier-ignore)' -- '*.js' '*.mjs' '*.cjs' '*.jsx' '*.ts' '*.mts' '*.cts' '*.tsx' ':!.core-services' || true; } | awk 'END { print NR }') echo "Inline ESLint and Prettier ignores: $inline" echo "count=$inline" >> "$GITHUB_OUTPUT" diff --git a/.github/workflows/run_markdown_checks.yml b/.github/workflows/run_markdown_checks.yml index 885082f..718e9e1 100644 --- a/.github/workflows/run_markdown_checks.yml +++ b/.github/workflows/run_markdown_checks.yml @@ -17,7 +17,7 @@ on: description: Outcome of the lint step (success, failure, cancelled or skipped) value: ${{ jobs.checks.outputs.lint }} baselined: - description: Number of inline markdownlint-disable comments in Markdown files + description: Number of inline markdownlint-disable directives in Markdown files value: ${{ jobs.checks.outputs.baselined }} permissions: {} @@ -51,6 +51,19 @@ jobs: registry-url: https://npm.pkg.github.com scope: '@opensesame' + - name: Set up npm + id: npm + run: | # zizmor: ignore[adhoc-packages] Installs the npm version the repository pins in packageManager; setup-node has no npm version input. + package_manager=$(jq -r 'if (.packageManager | type) == "string" then .packageManager else "" end' package.json) + if [[ "$package_manager" =~ ^npm@([0-9]+\.[0-9]+\.[0-9]+)(\+.*)?$ ]]; then + version=${BASH_REMATCH[1]} + echo "Installing npm $version from packageManager" + npm install --global "npm@$version" + else + echo "No npm@X.Y.Z packageManager; using the bundled npm" + fi + echo "npm $(npm --version)" + - name: Install dependencies id: install env: @@ -66,6 +79,6 @@ jobs: id: baseline if: ${{ !cancelled() }} run: | - inline=$({ git grep -c 'markdownlint-disabl[e]' -- '*.md' ':!.core-services' || true; } | awk -F: '{ sum += $NF } END { print sum + 0 }') + inline=$({ git grep -ohE '(^|[[:space:]])