From 724daa593bd14f506faf6e4bfe2483633cc7db41 Mon Sep 17 00:00:00 2001 From: Giuseppe Guerra Date: Tue, 12 May 2026 11:22:12 +0200 Subject: [PATCH 1/4] ignore node package manager scripts before installing packages --- .github/workflows/cd.yml | 10 ++++++ .github/workflows/ci.yml | 13 ++++++- .github/workflows/playwright.yml | 37 +++++++++++++++++++- actions/internal/plugins/frontend/action.yml | 36 ++++++++++++++++++- actions/internal/plugins/setup/action.yml | 3 ++ 5 files changed, 96 insertions(+), 3 deletions(-) diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index acf7bf05a..3f47cb26e 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -95,6 +95,15 @@ on: type: boolean required: false default: false + node-package-manager-allow-scripts: + description: | + Allow npm/yarn/pnpm lifecycle scripts (preinstall, install, postinstall, prepare, etc.) + to run during dependency installation. + Disabled by default for security. Only enable as a last resort if a plugin truly + requires install-time scripts and cannot work without them. + type: boolean + required: false + default: false allow-unsigned: description: | Allow to publish unsigned plugins. @@ -757,6 +766,7 @@ jobs: plugin-directory: ${{ inputs.plugin-directory }} plugin-version-suffix: ${{ needs.setup.outputs.plugin-version-suffix }} npm-registry-auth: ${{ inputs.npm-registry-auth }} + node-package-manager-allow-scripts: ${{ inputs.node-package-manager-allow-scripts }} go-version: ${{ inputs.go-version }} go-setup-caching: ${{ inputs.go-setup-caching }} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2893dcde5..dbb808a9f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -72,6 +72,15 @@ on: type: boolean required: false default: false + node-package-manager-allow-scripts: + description: | + Allow npm/yarn/pnpm lifecycle scripts (preinstall, install, postinstall, prepare, etc.) + to run during dependency installation. + Disabled by default for security. Only enable as a last resort if a plugin truly + requires install-time scripts and cannot work without them. + type: boolean + required: false + default: false # Playwright run-playwright: @@ -490,7 +499,7 @@ jobs: - name: Spellcheck run: | if [ -f "./cspell.config.json" ]; then - npx --yes cspell@6.13.3 -c cspell.config.json "**/*.{ts,tsx,js,go,md,mdx,yml,yaml,json,scss,css}" + npx --yes --ignore-scripts cspell@6.13.3 -c cspell.config.json "**/*.{ts,tsx,js,go,md,mdx,yml,yaml,json,scss,css}" else echo "❌ Spellcheck is not configured." fi @@ -534,6 +543,7 @@ jobs: plugin-directory: ${{ inputs.plugin-directory }} secrets: ${{ (fromJson(steps.workflow-context.outputs.result).isTrusted && inputs.frontend-secrets != '') && inputs.frontend-secrets || '' }} npm-registry-auth: ${{ inputs.npm-registry-auth }} + allow-scripts: ${{ inputs.node-package-manager-allow-scripts }} - name: Test and build backend id: backend @@ -741,6 +751,7 @@ jobs: secrets: ${{ (fromJson(needs.test-and-build.outputs.workflow-context).isTrusted && inputs.playwright-secrets != '') && inputs.playwright-secrets || '' }} node-version: ${{ inputs.node-version }} npm-registry-auth: ${{ inputs.npm-registry-auth }} + node-package-manager-allow-scripts: ${{ inputs.node-package-manager-allow-scripts }} playwright-docker: name: Plugins - Dockerized Playwright E2E tests diff --git a/.github/workflows/playwright.yml b/.github/workflows/playwright.yml index 7d03092f5..f22968f9b 100644 --- a/.github/workflows/playwright.yml +++ b/.github/workflows/playwright.yml @@ -83,6 +83,15 @@ on: type: boolean required: false default: false + node-package-manager-allow-scripts: + description: | + Allow npm/yarn/pnpm lifecycle scripts (preinstall, install, postinstall, prepare, etc.) + to run during dependency installation. + Disabled by default for security. Only enable as a last resort if a plugin truly + requires install-time scripts and cannot work without them. + type: boolean + required: false + default: false permissions: contents: read id-token: write @@ -146,8 +155,34 @@ jobs: working-directory: ${{ inputs.plugin-directory }} run: GOOGLE_APPLICATION_CREDENTIALS=${{ env.GOOGLE_APPLICATION_CREDENTIALS }} npx google-artifactregistry-auth --credential-config ./.npmrc + - name: Configure package manager script policy + id: install-cmd + shell: bash + run: | + INSTALL_CMD="${FROZEN_INSTALL_CMD}" + if [ "${ALLOW_SCRIPTS}" != "true" ]; then + # Yarn Berry does not support --ignore-scripts on the CLI (it errors with YN0050). + # For Yarn Berry, scripts are suppressed via the YARN_ENABLE_SCRIPTS env var. + # npm, pnpm, and Yarn Classic (agent=yarn) all accept --ignore-scripts on the CLI. + if [ "${PACKAGE_MANAGER_AGENT}" != "yarnBerry" ]; then + INSTALL_CMD="${INSTALL_CMD} --ignore-scripts" + fi + # Belt-and-suspenders: also export env vars so that scripts are blocked even if a + # wrapper or sub-process re-invokes the package manager without our CLI flags. + # npm and pnpm both honour NPM_CONFIG_IGNORE_SCRIPTS. + # Yarn Berry honours YARN_ENABLE_SCRIPTS (its env-var form of enableScripts config). + # Yarn Classic also honours npm_config_ignore_scripts. + echo "NPM_CONFIG_IGNORE_SCRIPTS=true" >> "$GITHUB_ENV" + echo "YARN_ENABLE_SCRIPTS=false" >> "$GITHUB_ENV" + fi + echo "cmd=${INSTALL_CMD}" >> "$GITHUB_OUTPUT" + env: + FROZEN_INSTALL_CMD: ${{ steps.setup.outputs.node-package-manager-frozen-install-cmd }} + PACKAGE_MANAGER_AGENT: ${{ steps.setup.outputs.node-package-manager-agent }} + ALLOW_SCRIPTS: ${{ inputs.node-package-manager-allow-scripts }} + - name: Install npm dependencies - run: ${{ steps.setup.outputs.node-package-manager-frozen-install-cmd }} + run: ${{ steps.install-cmd.outputs.cmd }} shell: bash working-directory: ${{ inputs.plugin-directory }} diff --git a/actions/internal/plugins/frontend/action.yml b/actions/internal/plugins/frontend/action.yml index 43bd0e6a0..d9e14b473 100644 --- a/actions/internal/plugins/frontend/action.yml +++ b/actions/internal/plugins/frontend/action.yml @@ -16,6 +16,14 @@ inputs: If true, the root of the plugin repository must contain a `.npmrc` file. required: false default: "false" + allow-scripts: + description: | + Allow npm/yarn/pnpm lifecycle scripts (preinstall, install, postinstall, prepare, etc.) + to run during dependency installation. + Disabled by default for security. Only enable as a last resort if a plugin truly + requires install-time scripts and cannot work without them. + required: false + default: "false" runs: using: composite @@ -47,10 +55,36 @@ runs: with: working-directory: ${{ inputs.plugin-directory }} + - name: Configure package manager script policy + id: install-cmd + shell: bash + run: | + INSTALL_CMD="${FROZEN_INSTALL_CMD}" + if [ "${ALLOW_SCRIPTS}" != "true" ]; then + # Yarn Berry does not support --ignore-scripts on the CLI (it errors with YN0050). + # For Yarn Berry, scripts are suppressed via the YARN_ENABLE_SCRIPTS env var. + # npm, pnpm, and Yarn Classic (agent=yarn) all accept --ignore-scripts on the CLI. + if [ "${PACKAGE_MANAGER_AGENT}" != "yarnBerry" ]; then + INSTALL_CMD="${INSTALL_CMD} --ignore-scripts" + fi + # Belt-and-suspenders: also export env vars so that scripts are blocked even if a + # wrapper or sub-process re-invokes the package manager without our CLI flags. + # npm and pnpm both honour NPM_CONFIG_IGNORE_SCRIPTS. + # Yarn Berry honours YARN_ENABLE_SCRIPTS (its env-var form of enableScripts config). + # Yarn Classic also honours npm_config_ignore_scripts. + echo "NPM_CONFIG_IGNORE_SCRIPTS=true" >> "$GITHUB_ENV" + echo "YARN_ENABLE_SCRIPTS=false" >> "$GITHUB_ENV" + fi + echo "cmd=${INSTALL_CMD}" >> "$GITHUB_OUTPUT" + env: + FROZEN_INSTALL_CMD: ${{ steps.package-manager.outputs.frozenInstallCmd }} + PACKAGE_MANAGER_AGENT: ${{ steps.package-manager.outputs.agent }} + ALLOW_SCRIPTS: ${{ inputs.allow-scripts }} + - name: Install dependencies shell: bash working-directory: ${{ inputs.plugin-directory }} - run: ${{ steps.package-manager.outputs.frozenInstallCmd }} + run: ${{ steps.install-cmd.outputs.cmd }} - name: Lint shell: bash diff --git a/actions/internal/plugins/setup/action.yml b/actions/internal/plugins/setup/action.yml index eea2cd47a..a3012fda4 100644 --- a/actions/internal/plugins/setup/action.yml +++ b/actions/internal/plugins/setup/action.yml @@ -76,6 +76,9 @@ outputs: node-package-manager-exec-local-cmd: description: The command to execute a locally installed package (e.g. npx, yarn dlx, pnpm exec). value: ${{ steps.package-manager.outputs.execLocalCmd }} + node-package-manager-agent: + description: The agent identifier for the detected Node.js package manager (e.g. npm, pnpm, yarn, yarnBerry). + value: ${{ steps.package-manager.outputs.agent }} runs: using: composite From c07db14cc8f0759427e62ac979a99a06ce7e9c0c Mon Sep 17 00:00:00 2001 From: Giuseppe Guerra Date: Tue, 12 May 2026 12:10:45 +0200 Subject: [PATCH 2/4] add act tests --- tests/act/internal/workflow/ci/ci.go | 4 + tests/act/main_install_scripts_test.go | 244 +++++++++++++++++++++++++ 2 files changed, 248 insertions(+) create mode 100644 tests/act/main_install_scripts_test.go diff --git a/tests/act/internal/workflow/ci/ci.go b/tests/act/internal/workflow/ci/ci.go index 4b5f10ab8..2d1bfcd50 100644 --- a/tests/act/internal/workflow/ci/ci.go +++ b/tests/act/internal/workflow/ci/ci.go @@ -113,6 +113,8 @@ type WorkflowInputs struct { Testing *bool BackendBuildTarget *string + NodePackageManagerAllowScripts *bool + DistArtifactsRetentionDays *int } @@ -141,6 +143,8 @@ func SetCIInputs(dst *workflow.Job, inputs WorkflowInputs) { workflow.SetJobInput(dst, "testing", inputs.Testing) workflow.SetJobInput(dst, "backend-build-target", inputs.BackendBuildTarget) + workflow.SetJobInput(dst, "node-package-manager-allow-scripts", inputs.NodePackageManagerAllowScripts) + workflow.SetJobInput(dst, "dist-artifacts-retention-days", inputs.DistArtifactsRetentionDays) } diff --git a/tests/act/main_install_scripts_test.go b/tests/act/main_install_scripts_test.go new file mode 100644 index 000000000..0784dfd51 --- /dev/null +++ b/tests/act/main_install_scripts_test.go @@ -0,0 +1,244 @@ +package main + +import ( + "fmt" + "path/filepath" + "strings" + "testing" + + "github.com/grafana/plugin-ci-workflows/tests/act/internal/act" + "github.com/grafana/plugin-ci-workflows/tests/act/internal/workflow" + "github.com/grafana/plugin-ci-workflows/tests/act/internal/workflow/ci" + "github.com/stretchr/testify/require" +) + +// TestInstallScripts verifies the "Configure package manager script policy" logic +// that is applied before running npm/yarn/pnpm install in the frontend action and +// playwright workflow. +// +// It tests the two dimensions that matter: +// 1. The package manager agent (npm, pnpm, yarn, yarnBerry) - determines whether +// --ignore-scripts can be appended to the CLI install command. +// 2. The node-package-manager-allow-scripts input (true/false) - determines whether +// scripts should be disabled at all. +// +// Since the "Configure package manager script policy" step lives inside the +// frontend composite action (which we cannot inject into directly via the test +// framework), we replace the entire `frontend` step in test-and-build with an +// inline bash step that reproduces the same logic and emits ::act-debug:: +// annotations. This lets us assert on the computed install command and the env +// vars that get exported to GITHUB_ENV, without running the real install. +func TestInstallScripts(t *testing.T) { + // injectScriptPolicyStep is a workflow step that reproduces the + // "Configure package manager script policy" shell logic from + // actions/internal/plugins/frontend/action.yml, emitting ::act-debug:: + // annotations so the test framework can capture and assert on the results. + // + // Emitted annotations (logfmt format): + // msg="install-cmd" cmd= + // msg="npm-config-ignore-scripts" value= + // msg="yarn-enable-scripts" value= + injectScriptPolicyStep := func(frozenInstallCmd, packageManagerAgent string, allowScripts bool) workflow.Step { + allowScriptsStr := "false" + if allowScripts { + allowScriptsStr = "true" + } + return workflow.Step{ + Name: "Configure package manager script policy (test probe)", + Shell: "bash", + Run: workflow.Commands{ + `INSTALL_CMD="${FROZEN_INSTALL_CMD}"`, + `if [ "${ALLOW_SCRIPTS}" != "true" ]; then`, + ` if [ "${PACKAGE_MANAGER_AGENT}" != "yarnBerry" ]; then`, + ` INSTALL_CMD="${INSTALL_CMD} --ignore-scripts"`, + ` fi`, + ` echo "NPM_CONFIG_IGNORE_SCRIPTS=true" >> "$GITHUB_ENV"`, + ` echo "YARN_ENABLE_SCRIPTS=false" >> "$GITHUB_ENV"`, + ` printf '::act-debug::msg="npm-config-ignore-scripts" value=true\n'`, + ` printf '::act-debug::msg="yarn-enable-scripts" value=false\n'`, + `else`, + ` printf '::act-debug::msg="npm-config-ignore-scripts" value=not-set\n'`, + ` printf '::act-debug::msg="yarn-enable-scripts" value=not-set\n'`, + `fi`, + `printf '::act-debug::msg="install-cmd" cmd=%s\n' "${INSTALL_CMD}"`, + }.String(), + Env: map[string]string{ + "FROZEN_INSTALL_CMD": frozenInstallCmd, + "PACKAGE_MANAGER_AGENT": packageManagerAgent, + "ALLOW_SCRIPTS": allowScriptsStr, + }, + } + } + + type testCase struct { + name string + packageManagerAgent string + frozenInstallCmd string + allowScripts bool + wantCmdContains string + wantCmdNotContains string + wantNPMIgnore bool // true = NPM_CONFIG_IGNORE_SCRIPTS=true expected + wantYarnDisable bool // true = YARN_ENABLE_SCRIPTS=false expected + } + + testCases := []testCase{ + // --- scripts disabled (default) --- + { + name: "npm / scripts disabled", + packageManagerAgent: "npm", + frozenInstallCmd: "npm ci", + allowScripts: false, + wantCmdContains: "--ignore-scripts", + wantNPMIgnore: true, + wantYarnDisable: true, + }, + { + name: "pnpm / scripts disabled", + packageManagerAgent: "pnpm", + frozenInstallCmd: "pnpm install --frozen-lockfile", + allowScripts: false, + wantCmdContains: "--ignore-scripts", + wantNPMIgnore: true, + wantYarnDisable: true, + }, + { + name: "yarn classic / scripts disabled", + packageManagerAgent: "yarn", + frozenInstallCmd: "yarn install --frozen-lockfile", + allowScripts: false, + wantCmdContains: "--ignore-scripts", + wantNPMIgnore: true, + wantYarnDisable: true, + }, + { + // Yarn Berry does not support --ignore-scripts on the CLI. + // Scripts are suppressed only via YARN_ENABLE_SCRIPTS env var. + name: "yarn berry / scripts disabled", + packageManagerAgent: "yarnBerry", + frozenInstallCmd: "yarn install --immutable", + allowScripts: false, + wantCmdNotContains: "--ignore-scripts", + wantNPMIgnore: true, + wantYarnDisable: true, + }, + + // --- scripts enabled (allow-scripts = true) --- + { + name: "npm / scripts allowed", + packageManagerAgent: "npm", + frozenInstallCmd: "npm ci", + allowScripts: true, + wantCmdNotContains: "--ignore-scripts", + wantNPMIgnore: false, + wantYarnDisable: false, + }, + { + name: "pnpm / scripts allowed", + packageManagerAgent: "pnpm", + frozenInstallCmd: "pnpm install --frozen-lockfile", + allowScripts: true, + wantCmdNotContains: "--ignore-scripts", + wantNPMIgnore: false, + wantYarnDisable: false, + }, + { + name: "yarn classic / scripts allowed", + packageManagerAgent: "yarn", + frozenInstallCmd: "yarn install --frozen-lockfile", + allowScripts: true, + wantCmdNotContains: "--ignore-scripts", + wantNPMIgnore: false, + wantYarnDisable: false, + }, + { + name: "yarn berry / scripts allowed", + packageManagerAgent: "yarnBerry", + frozenInstallCmd: "yarn install --immutable", + allowScripts: true, + wantCmdNotContains: "--ignore-scripts", + wantNPMIgnore: false, + wantYarnDisable: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + runner, err := act.NewRunner(t) + require.NoError(t, err) + + probeStep := injectScriptPolicyStep(tc.frozenInstallCmd, tc.packageManagerAgent, tc.allowScripts) + + wf, err := ci.NewWorkflow( + ci.WithWorkflowInputs(ci.WorkflowInputs{ + PluginDirectory: workflow.Input(filepath.Join("tests", "simple-frontend")), + DistArtifactsPrefix: workflow.Input("simple-frontend-"), + RunPlaywright: workflow.Input(false), + NodePackageManagerAllowScripts: workflow.Input(tc.allowScripts), + }), + ci.MutateCIWorkflow().With( + // Only run the job we care about, drop all others (playwright, docs, GCS upload, etc.) + workflow.WithOnlyOneJob(t, "test-and-build", true), + // Replace the frontend composite action step with our inline probe step. + // This avoids a real npm install while still exercising the script-policy logic. + // Note: WithReplacedStep preserves the original step ID ("frontend"), so we must + // reference "frontend" (not the probe step's own ID) in WithRemoveAllStepsAfter. + workflow.WithReplacedStep(t, "test-and-build", "frontend", probeStep), + // Stop after our probe step (which kept the ID "frontend") — no need to run + // package/sign/upload steps. + workflow.WithRemoveAllStepsAfter(t, "test-and-build", "frontend"), + ), + ) + require.NoError(t, err) + + r, err := runner.Run(wf, act.NewPushEventPayload("main")) + require.NoError(t, err) + require.True(t, r.Success, "workflow should succeed") + + // --- Assert install command --- + // The probe step emits: ::act-debug::msg="install-cmd" cmd= + // We find the annotation by prefix and assert on its content. + var installCmdAnnotation *act.Annotation + for i, ann := range r.Annotations { + if ann.Level == act.AnnotationLevelDebug && strings.Contains(ann.Message, `msg="install-cmd"`) { + installCmdAnnotation = &r.Annotations[i] + break + } + } + require.NotNil(t, installCmdAnnotation, "install-cmd annotation should be present") + if tc.wantCmdContains != "" { + require.Contains(t, installCmdAnnotation.Message, tc.wantCmdContains, + "install command annotation should contain %q", tc.wantCmdContains) + } + if tc.wantCmdNotContains != "" { + require.NotContains(t, installCmdAnnotation.Message, tc.wantCmdNotContains, + "install command annotation should not contain %q", tc.wantCmdNotContains) + } + + // --- Assert env var annotations --- + // The probe step emits these in hardcoded logfmt-like format with quoted keys: + // msg="npm-config-ignore-scripts" value= + // msg="yarn-enable-scripts" value= + npmIgnoreVal := "not-set" + if tc.wantNPMIgnore { + npmIgnoreVal = "true" + } + expectedNPMMsg := fmt.Sprintf(`msg="npm-config-ignore-scripts" value=%s`, npmIgnoreVal) + require.Contains(t, r.Annotations, act.Annotation{ + Level: act.AnnotationLevelDebug, + Message: expectedNPMMsg, + }, "NPM_CONFIG_IGNORE_SCRIPTS annotation should have value=%s", npmIgnoreVal) + + yarnVal := "not-set" + if tc.wantYarnDisable { + yarnVal = "false" + } + expectedYarnMsg := fmt.Sprintf(`msg="yarn-enable-scripts" value=%s`, yarnVal) + require.Contains(t, r.Annotations, act.Annotation{ + Level: act.AnnotationLevelDebug, + Message: expectedYarnMsg, + }, "YARN_ENABLE_SCRIPTS annotation should have value=%s", yarnVal) + }) + } +} From e39f49dc58f3bca6a1155250e5f74afb3f10fb2a Mon Sep 17 00:00:00 2001 From: Giuseppe Guerra Date: Tue, 12 May 2026 12:18:44 +0200 Subject: [PATCH 3/4] simplify act tests --- .github/workflows/playwright.yml | 6 + actions/internal/plugins/frontend/action.yml | 6 + tests/act/main_install_scripts_test.go | 234 +++++++------------ 3 files changed, 90 insertions(+), 156 deletions(-) diff --git a/.github/workflows/playwright.yml b/.github/workflows/playwright.yml index f22968f9b..1244d9059 100644 --- a/.github/workflows/playwright.yml +++ b/.github/workflows/playwright.yml @@ -174,8 +174,14 @@ jobs: # Yarn Classic also honours npm_config_ignore_scripts. echo "NPM_CONFIG_IGNORE_SCRIPTS=true" >> "$GITHUB_ENV" echo "YARN_ENABLE_SCRIPTS=false" >> "$GITHUB_ENV" + printf '::act-debug::msg="npm-config-ignore-scripts" value=true\n' + printf '::act-debug::msg="yarn-enable-scripts" value=false\n' + else + printf '::act-debug::msg="npm-config-ignore-scripts" value=not-set\n' + printf '::act-debug::msg="yarn-enable-scripts" value=not-set\n' fi echo "cmd=${INSTALL_CMD}" >> "$GITHUB_OUTPUT" + printf '::act-debug::msg="install-cmd" cmd=%s\n' "${INSTALL_CMD}" env: FROZEN_INSTALL_CMD: ${{ steps.setup.outputs.node-package-manager-frozen-install-cmd }} PACKAGE_MANAGER_AGENT: ${{ steps.setup.outputs.node-package-manager-agent }} diff --git a/actions/internal/plugins/frontend/action.yml b/actions/internal/plugins/frontend/action.yml index d9e14b473..5076065fc 100644 --- a/actions/internal/plugins/frontend/action.yml +++ b/actions/internal/plugins/frontend/action.yml @@ -74,8 +74,14 @@ runs: # Yarn Classic also honours npm_config_ignore_scripts. echo "NPM_CONFIG_IGNORE_SCRIPTS=true" >> "$GITHUB_ENV" echo "YARN_ENABLE_SCRIPTS=false" >> "$GITHUB_ENV" + printf '::act-debug::msg="npm-config-ignore-scripts" value=true\n' + printf '::act-debug::msg="yarn-enable-scripts" value=false\n' + else + printf '::act-debug::msg="npm-config-ignore-scripts" value=not-set\n' + printf '::act-debug::msg="yarn-enable-scripts" value=not-set\n' fi echo "cmd=${INSTALL_CMD}" >> "$GITHUB_OUTPUT" + printf '::act-debug::msg="install-cmd" cmd=%s\n' "${INSTALL_CMD}" env: FROZEN_INSTALL_CMD: ${{ steps.package-manager.outputs.frozenInstallCmd }} PACKAGE_MANAGER_AGENT: ${{ steps.package-manager.outputs.agent }} diff --git a/tests/act/main_install_scripts_test.go b/tests/act/main_install_scripts_test.go index 0784dfd51..6dab55bdb 100644 --- a/tests/act/main_install_scripts_test.go +++ b/tests/act/main_install_scripts_test.go @@ -13,180 +13,103 @@ import ( ) // TestInstallScripts verifies the "Configure package manager script policy" logic -// that is applied before running npm/yarn/pnpm install in the frontend action and -// playwright workflow. +// in actions/internal/plugins/frontend/action.yml by running the real frontend +// composite action against the test plugins and asserting on the ::act-debug:: +// annotations it emits. // -// It tests the two dimensions that matter: -// 1. The package manager agent (npm, pnpm, yarn, yarnBerry) - determines whether -// --ignore-scripts can be appended to the CLI install command. -// 2. The node-package-manager-allow-scripts input (true/false) - determines whether -// scripts should be disabled at all. +// The annotations are emitted directly by the production "Configure package manager +// script policy" step in the frontend action: // -// Since the "Configure package manager script policy" step lives inside the -// frontend composite action (which we cannot inject into directly via the test -// framework), we replace the entire `frontend` step in test-and-build with an -// inline bash step that reproduces the same logic and emits ::act-debug:: -// annotations. This lets us assert on the computed install command and the env -// vars that get exported to GITHUB_ENV, without running the real install. +// msg="install-cmd" cmd= +// msg="npm-config-ignore-scripts" value= +// msg="yarn-enable-scripts" value= +// +// We use the three real test plugins so that package-manager-detect runs for real +// and produces the correct agent/frozenInstallCmd values: +// - tests/simple-frontend → npm (npm ci) +// - tests/simple-frontend-yarn → yarn classic (yarn install --frozen-lockfile) +// - tests/simple-frontend-pnpm → pnpm (pnpm install --frozen-lockfile) +// +// For each plugin we test both allow-scripts=false (default, secure) and +// allow-scripts=true (escape-hatch). func TestInstallScripts(t *testing.T) { - // injectScriptPolicyStep is a workflow step that reproduces the - // "Configure package manager script policy" shell logic from - // actions/internal/plugins/frontend/action.yml, emitting ::act-debug:: - // annotations so the test framework can capture and assert on the results. - // - // Emitted annotations (logfmt format): - // msg="install-cmd" cmd= - // msg="npm-config-ignore-scripts" value= - // msg="yarn-enable-scripts" value= - injectScriptPolicyStep := func(frozenInstallCmd, packageManagerAgent string, allowScripts bool) workflow.Step { - allowScriptsStr := "false" - if allowScripts { - allowScriptsStr = "true" - } - return workflow.Step{ - Name: "Configure package manager script policy (test probe)", - Shell: "bash", - Run: workflow.Commands{ - `INSTALL_CMD="${FROZEN_INSTALL_CMD}"`, - `if [ "${ALLOW_SCRIPTS}" != "true" ]; then`, - ` if [ "${PACKAGE_MANAGER_AGENT}" != "yarnBerry" ]; then`, - ` INSTALL_CMD="${INSTALL_CMD} --ignore-scripts"`, - ` fi`, - ` echo "NPM_CONFIG_IGNORE_SCRIPTS=true" >> "$GITHUB_ENV"`, - ` echo "YARN_ENABLE_SCRIPTS=false" >> "$GITHUB_ENV"`, - ` printf '::act-debug::msg="npm-config-ignore-scripts" value=true\n'`, - ` printf '::act-debug::msg="yarn-enable-scripts" value=false\n'`, - `else`, - ` printf '::act-debug::msg="npm-config-ignore-scripts" value=not-set\n'`, - ` printf '::act-debug::msg="yarn-enable-scripts" value=not-set\n'`, - `fi`, - `printf '::act-debug::msg="install-cmd" cmd=%s\n' "${INSTALL_CMD}"`, - }.String(), - Env: map[string]string{ - "FROZEN_INSTALL_CMD": frozenInstallCmd, - "PACKAGE_MANAGER_AGENT": packageManagerAgent, - "ALLOW_SCRIPTS": allowScriptsStr, - }, - } - } - type testCase struct { - name string - packageManagerAgent string - frozenInstallCmd string - allowScripts bool - wantCmdContains string - wantCmdNotContains string - wantNPMIgnore bool // true = NPM_CONFIG_IGNORE_SCRIPTS=true expected - wantYarnDisable bool // true = YARN_ENABLE_SCRIPTS=false expected + folder string + allowScripts bool + wantCmdContains string + wantCmdNotContains string + wantNPMIgnore bool // true → NPM_CONFIG_IGNORE_SCRIPTS=true emitted + wantYarnDisable bool // true → YARN_ENABLE_SCRIPTS=false emitted } - testCases := []testCase{ - // --- scripts disabled (default) --- - { - name: "npm / scripts disabled", - packageManagerAgent: "npm", - frozenInstallCmd: "npm ci", - allowScripts: false, - wantCmdContains: "--ignore-scripts", - wantNPMIgnore: true, - wantYarnDisable: true, - }, + for _, tc := range []testCase{ + // --- scripts disabled (default, secure) --- { - name: "pnpm / scripts disabled", - packageManagerAgent: "pnpm", - frozenInstallCmd: "pnpm install --frozen-lockfile", - allowScripts: false, - wantCmdContains: "--ignore-scripts", - wantNPMIgnore: true, - wantYarnDisable: true, + folder: "simple-frontend", + allowScripts: false, + wantCmdContains: "--ignore-scripts", + wantNPMIgnore: true, + wantYarnDisable: true, }, { - name: "yarn classic / scripts disabled", - packageManagerAgent: "yarn", - frozenInstallCmd: "yarn install --frozen-lockfile", - allowScripts: false, - wantCmdContains: "--ignore-scripts", - wantNPMIgnore: true, - wantYarnDisable: true, + folder: "simple-frontend-yarn", + allowScripts: false, + wantCmdContains: "--ignore-scripts", + wantNPMIgnore: true, + wantYarnDisable: true, }, { - // Yarn Berry does not support --ignore-scripts on the CLI. - // Scripts are suppressed only via YARN_ENABLE_SCRIPTS env var. - name: "yarn berry / scripts disabled", - packageManagerAgent: "yarnBerry", - frozenInstallCmd: "yarn install --immutable", - allowScripts: false, - wantCmdNotContains: "--ignore-scripts", - wantNPMIgnore: true, - wantYarnDisable: true, + folder: "simple-frontend-pnpm", + allowScripts: false, + wantCmdContains: "--ignore-scripts", + wantNPMIgnore: true, + wantYarnDisable: true, }, - // --- scripts enabled (allow-scripts = true) --- + // --- scripts enabled (allow-scripts = true, escape-hatch) --- { - name: "npm / scripts allowed", - packageManagerAgent: "npm", - frozenInstallCmd: "npm ci", - allowScripts: true, - wantCmdNotContains: "--ignore-scripts", - wantNPMIgnore: false, - wantYarnDisable: false, + folder: "simple-frontend", + allowScripts: true, + wantCmdNotContains: "--ignore-scripts", + wantNPMIgnore: false, + wantYarnDisable: false, }, { - name: "pnpm / scripts allowed", - packageManagerAgent: "pnpm", - frozenInstallCmd: "pnpm install --frozen-lockfile", - allowScripts: true, - wantCmdNotContains: "--ignore-scripts", - wantNPMIgnore: false, - wantYarnDisable: false, + folder: "simple-frontend-yarn", + allowScripts: true, + wantCmdNotContains: "--ignore-scripts", + wantNPMIgnore: false, + wantYarnDisable: false, }, { - name: "yarn classic / scripts allowed", - packageManagerAgent: "yarn", - frozenInstallCmd: "yarn install --frozen-lockfile", - allowScripts: true, - wantCmdNotContains: "--ignore-scripts", - wantNPMIgnore: false, - wantYarnDisable: false, + folder: "simple-frontend-pnpm", + allowScripts: true, + wantCmdNotContains: "--ignore-scripts", + wantNPMIgnore: false, + wantYarnDisable: false, }, - { - name: "yarn berry / scripts allowed", - packageManagerAgent: "yarnBerry", - frozenInstallCmd: "yarn install --immutable", - allowScripts: true, - wantCmdNotContains: "--ignore-scripts", - wantNPMIgnore: false, - wantYarnDisable: false, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { + } { + name := fmt.Sprintf("%s/allow-scripts=%v", tc.folder, tc.allowScripts) + t.Run(name, func(t *testing.T) { t.Parallel() runner, err := act.NewRunner(t) require.NoError(t, err) - probeStep := injectScriptPolicyStep(tc.frozenInstallCmd, tc.packageManagerAgent, tc.allowScripts) - wf, err := ci.NewWorkflow( ci.WithWorkflowInputs(ci.WorkflowInputs{ - PluginDirectory: workflow.Input(filepath.Join("tests", "simple-frontend")), - DistArtifactsPrefix: workflow.Input("simple-frontend-"), - RunPlaywright: workflow.Input(false), + PluginDirectory: workflow.Input(filepath.Join("tests", tc.folder)), + DistArtifactsPrefix: workflow.Input(tc.folder + "-"), + RunPlaywright: workflow.Input(false), NodePackageManagerAllowScripts: workflow.Input(tc.allowScripts), }), ci.MutateCIWorkflow().With( - // Only run the job we care about, drop all others (playwright, docs, GCS upload, etc.) + // Only run the job we care about. workflow.WithOnlyOneJob(t, "test-and-build", true), - // Replace the frontend composite action step with our inline probe step. - // This avoids a real npm install while still exercising the script-policy logic. - // Note: WithReplacedStep preserves the original step ID ("frontend"), so we must - // reference "frontend" (not the probe step's own ID) in WithRemoveAllStepsAfter. - workflow.WithReplacedStep(t, "test-and-build", "frontend", probeStep), - // Stop after our probe step (which kept the ID "frontend") — no need to run - // package/sign/upload steps. + // Stop right after the frontend step completes — the ::act-debug:: + // annotations we assert on are emitted inside the frontend composite + // action's "Configure package manager script policy" step. We don't + // need the backend, packaging, or upload steps. workflow.WithRemoveAllStepsAfter(t, "test-and-build", "frontend"), ), ) @@ -196,9 +119,8 @@ func TestInstallScripts(t *testing.T) { require.NoError(t, err) require.True(t, r.Success, "workflow should succeed") - // --- Assert install command --- - // The probe step emits: ::act-debug::msg="install-cmd" cmd= - // We find the annotation by prefix and assert on its content. + // --- Assert install-cmd annotation --- + // The frontend action emits: ::act-debug::msg="install-cmd" cmd= var installCmdAnnotation *act.Annotation for i, ann := range r.Annotations { if ann.Level == act.AnnotationLevelDebug && strings.Contains(ann.Message, `msg="install-cmd"`) { @@ -209,35 +131,35 @@ func TestInstallScripts(t *testing.T) { require.NotNil(t, installCmdAnnotation, "install-cmd annotation should be present") if tc.wantCmdContains != "" { require.Contains(t, installCmdAnnotation.Message, tc.wantCmdContains, - "install command annotation should contain %q", tc.wantCmdContains) + "install command should contain %q", tc.wantCmdContains) } if tc.wantCmdNotContains != "" { require.NotContains(t, installCmdAnnotation.Message, tc.wantCmdNotContains, - "install command annotation should not contain %q", tc.wantCmdNotContains) + "install command should not contain %q", tc.wantCmdNotContains) } // --- Assert env var annotations --- - // The probe step emits these in hardcoded logfmt-like format with quoted keys: - // msg="npm-config-ignore-scripts" value= - // msg="yarn-enable-scripts" value= + // The frontend action emits one of: + // ::act-debug::msg="npm-config-ignore-scripts" value=true + // ::act-debug::msg="npm-config-ignore-scripts" value=not-set npmIgnoreVal := "not-set" if tc.wantNPMIgnore { npmIgnoreVal = "true" } - expectedNPMMsg := fmt.Sprintf(`msg="npm-config-ignore-scripts" value=%s`, npmIgnoreVal) require.Contains(t, r.Annotations, act.Annotation{ Level: act.AnnotationLevelDebug, - Message: expectedNPMMsg, + Message: fmt.Sprintf(`msg="npm-config-ignore-scripts" value=%s`, npmIgnoreVal), }, "NPM_CONFIG_IGNORE_SCRIPTS annotation should have value=%s", npmIgnoreVal) + // ::act-debug::msg="yarn-enable-scripts" value=false + // ::act-debug::msg="yarn-enable-scripts" value=not-set yarnVal := "not-set" if tc.wantYarnDisable { yarnVal = "false" } - expectedYarnMsg := fmt.Sprintf(`msg="yarn-enable-scripts" value=%s`, yarnVal) require.Contains(t, r.Annotations, act.Annotation{ Level: act.AnnotationLevelDebug, - Message: expectedYarnMsg, + Message: fmt.Sprintf(`msg="yarn-enable-scripts" value=%s`, yarnVal), }, "YARN_ENABLE_SCRIPTS annotation should have value=%s", yarnVal) }) } From f8908e915401b7f9ae2e1da9cf4fbf9a37aec0b4 Mon Sep 17 00:00:00 2001 From: Giuseppe Guerra Date: Tue, 12 May 2026 12:21:17 +0200 Subject: [PATCH 4/4] remove unnecessary comments --- tests/act/main_install_scripts_test.go | 104 +++++-------------------- 1 file changed, 18 insertions(+), 86 deletions(-) diff --git a/tests/act/main_install_scripts_test.go b/tests/act/main_install_scripts_test.go index 6dab55bdb..5baaa1c61 100644 --- a/tests/act/main_install_scripts_test.go +++ b/tests/act/main_install_scripts_test.go @@ -13,84 +13,31 @@ import ( ) // TestInstallScripts verifies the "Configure package manager script policy" logic -// in actions/internal/plugins/frontend/action.yml by running the real frontend -// composite action against the test plugins and asserting on the ::act-debug:: -// annotations it emits. +// in the frontend action against the three real test plugins (npm, yarn, pnpm), +// with allow-scripts both true and false. // -// The annotations are emitted directly by the production "Configure package manager -// script policy" step in the frontend action: -// -// msg="install-cmd" cmd= -// msg="npm-config-ignore-scripts" value= -// msg="yarn-enable-scripts" value= -// -// We use the three real test plugins so that package-manager-detect runs for real -// and produces the correct agent/frozenInstallCmd values: -// - tests/simple-frontend → npm (npm ci) -// - tests/simple-frontend-yarn → yarn classic (yarn install --frozen-lockfile) -// - tests/simple-frontend-pnpm → pnpm (pnpm install --frozen-lockfile) -// -// For each plugin we test both allow-scripts=false (default, secure) and -// allow-scripts=true (escape-hatch). +// TODO: add a simple-frontend-yarn-berry test plugin and cover the yarnBerry agent path, +// where --ignore-scripts is NOT appended to the CLI (Berry rejects it) and scripts are +// suppressed solely via the YARN_ENABLE_SCRIPTS env var. func TestInstallScripts(t *testing.T) { type testCase struct { folder string allowScripts bool wantCmdContains string wantCmdNotContains string - wantNPMIgnore bool // true → NPM_CONFIG_IGNORE_SCRIPTS=true emitted - wantYarnDisable bool // true → YARN_ENABLE_SCRIPTS=false emitted + wantNPMIgnore bool + wantYarnDisable bool } for _, tc := range []testCase{ - // --- scripts disabled (default, secure) --- - { - folder: "simple-frontend", - allowScripts: false, - wantCmdContains: "--ignore-scripts", - wantNPMIgnore: true, - wantYarnDisable: true, - }, - { - folder: "simple-frontend-yarn", - allowScripts: false, - wantCmdContains: "--ignore-scripts", - wantNPMIgnore: true, - wantYarnDisable: true, - }, - { - folder: "simple-frontend-pnpm", - allowScripts: false, - wantCmdContains: "--ignore-scripts", - wantNPMIgnore: true, - wantYarnDisable: true, - }, - - // --- scripts enabled (allow-scripts = true, escape-hatch) --- - { - folder: "simple-frontend", - allowScripts: true, - wantCmdNotContains: "--ignore-scripts", - wantNPMIgnore: false, - wantYarnDisable: false, - }, - { - folder: "simple-frontend-yarn", - allowScripts: true, - wantCmdNotContains: "--ignore-scripts", - wantNPMIgnore: false, - wantYarnDisable: false, - }, - { - folder: "simple-frontend-pnpm", - allowScripts: true, - wantCmdNotContains: "--ignore-scripts", - wantNPMIgnore: false, - wantYarnDisable: false, - }, + {folder: "simple-frontend", allowScripts: false, wantCmdContains: "--ignore-scripts", wantNPMIgnore: true, wantYarnDisable: true}, + {folder: "simple-frontend-yarn", allowScripts: false, wantCmdContains: "--ignore-scripts", wantNPMIgnore: true, wantYarnDisable: true}, + {folder: "simple-frontend-pnpm", allowScripts: false, wantCmdContains: "--ignore-scripts", wantNPMIgnore: true, wantYarnDisable: true}, + {folder: "simple-frontend", allowScripts: true, wantCmdNotContains: "--ignore-scripts", wantNPMIgnore: false, wantYarnDisable: false}, + {folder: "simple-frontend-yarn", allowScripts: true, wantCmdNotContains: "--ignore-scripts", wantNPMIgnore: false, wantYarnDisable: false}, + {folder: "simple-frontend-pnpm", allowScripts: true, wantCmdNotContains: "--ignore-scripts", wantNPMIgnore: false, wantYarnDisable: false}, } { - name := fmt.Sprintf("%s/allow-scripts=%v", tc.folder, tc.allowScripts) - t.Run(name, func(t *testing.T) { + t.Run(fmt.Sprintf("%s/allow-scripts=%v", tc.folder, tc.allowScripts), func(t *testing.T) { t.Parallel() runner, err := act.NewRunner(t) @@ -104,12 +51,7 @@ func TestInstallScripts(t *testing.T) { NodePackageManagerAllowScripts: workflow.Input(tc.allowScripts), }), ci.MutateCIWorkflow().With( - // Only run the job we care about. workflow.WithOnlyOneJob(t, "test-and-build", true), - // Stop right after the frontend step completes — the ::act-debug:: - // annotations we assert on are emitted inside the frontend composite - // action's "Configure package manager script policy" step. We don't - // need the backend, packaging, or upload steps. workflow.WithRemoveAllStepsAfter(t, "test-and-build", "frontend"), ), ) @@ -119,8 +61,6 @@ func TestInstallScripts(t *testing.T) { require.NoError(t, err) require.True(t, r.Success, "workflow should succeed") - // --- Assert install-cmd annotation --- - // The frontend action emits: ::act-debug::msg="install-cmd" cmd= var installCmdAnnotation *act.Annotation for i, ann := range r.Annotations { if ann.Level == act.AnnotationLevelDebug && strings.Contains(ann.Message, `msg="install-cmd"`) { @@ -130,18 +70,12 @@ func TestInstallScripts(t *testing.T) { } require.NotNil(t, installCmdAnnotation, "install-cmd annotation should be present") if tc.wantCmdContains != "" { - require.Contains(t, installCmdAnnotation.Message, tc.wantCmdContains, - "install command should contain %q", tc.wantCmdContains) + require.Contains(t, installCmdAnnotation.Message, tc.wantCmdContains) } if tc.wantCmdNotContains != "" { - require.NotContains(t, installCmdAnnotation.Message, tc.wantCmdNotContains, - "install command should not contain %q", tc.wantCmdNotContains) + require.NotContains(t, installCmdAnnotation.Message, tc.wantCmdNotContains) } - // --- Assert env var annotations --- - // The frontend action emits one of: - // ::act-debug::msg="npm-config-ignore-scripts" value=true - // ::act-debug::msg="npm-config-ignore-scripts" value=not-set npmIgnoreVal := "not-set" if tc.wantNPMIgnore { npmIgnoreVal = "true" @@ -149,10 +83,8 @@ func TestInstallScripts(t *testing.T) { require.Contains(t, r.Annotations, act.Annotation{ Level: act.AnnotationLevelDebug, Message: fmt.Sprintf(`msg="npm-config-ignore-scripts" value=%s`, npmIgnoreVal), - }, "NPM_CONFIG_IGNORE_SCRIPTS annotation should have value=%s", npmIgnoreVal) + }) - // ::act-debug::msg="yarn-enable-scripts" value=false - // ::act-debug::msg="yarn-enable-scripts" value=not-set yarnVal := "not-set" if tc.wantYarnDisable { yarnVal = "false" @@ -160,7 +92,7 @@ func TestInstallScripts(t *testing.T) { require.Contains(t, r.Annotations, act.Annotation{ Level: act.AnnotationLevelDebug, Message: fmt.Sprintf(`msg="yarn-enable-scripts" value=%s`, yarnVal), - }, "YARN_ENABLE_SCRIPTS annotation should have value=%s", yarnVal) + }) }) } }