BUILD-10827: make config-npm skip path safe from template parsing error

[tomverin]

Summary

Fix noisy ##[error] annotations when config-npm is invoked a second time in the same job (e.g. via SonarSource/sonarqube-cloud-github-actions/parse-cdk-nag-output@master, which nests a call to config-npm).

Root cause. In config-npm/action.yml, the Configure NPM authentication step declares:

env:
  ARTIFACTORY_ACCESS_TOKEN: ${{ fromJSON(steps.secrets.outputs.vault).ARTIFACTORY_ACCESS_TOKEN }}

On the skip path (when CONFIG_NPM_COMPLETED is already set), the secrets step is gated out by if: steps.config-npm-completed.outputs.skip != 'true' and therefore does not produce any output. GitHub Actions still evaluates the step-level env expression, so fromJSON('') raises a template parsing error (Error reading JToken from JsonReader). The job may still pass, but the ##[error] annotation is reported and can hide real failures.

Fix. Make the expression null-safe using the exact same pattern already used in config-maven/action.yml, config-gradle/action.yml and get-build-number/action.yml:

ARTIFACTORY_ACCESS_TOKEN: ${{ steps.secrets.outputs.vault && fromJSON(steps.secrets.outputs.vault).ARTIFACTORY_ACCESS_TOKEN || '' }}

When steps.secrets.outputs.vault is empty (skip path), the expression short-circuits to '' and fromJSON is never called. First-invocation behaviour is unchanged because steps.secrets.outputs.vault is always a non-empty JSON string when the secrets step runs.

JIRA: https://sonarsource.atlassian.net/browse/BUILD-10827

Notes on test coverage

• The existing spec/config-npm_spec.sh ShellSpec suite covers the shell scripts (npm_config.sh, npm_set_project_version.sh), not the composite action's YAML. There is no existing harness for double-invocation of the composite action itself, so this fix is verified by workflow-level observation rather than unit tests.
• The equivalent null-safe pattern in config-maven/config-gradle was introduced for the same reason and has been stable since; this change brings config-npm in line with them.

Test plan

• Merge-queue / PR CI: test-shell-scripts.yml still passes (first-invocation path unchanged).
• Manual: trigger a workflow that calls config-npm twice in the same job (e.g. via SonarSource/sonarqube-cloud-github-actions/parse-cdk-nag-output) and confirm:
  • No ##[error]Error reading JToken from JsonReader annotation on the second call.
  • Second call logs Action already called by ..., execution skipped. and exits cleanly.
  • First call still configures ~/.npmrc and jf correctly with the Artifactory token.

[hashicorp-vault-sonar-prod]

BUILD-10827

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues
0 New dependency risks

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[julien-carsique-sonarsource]

There is no existing harness for double-invocation of the composite action itself, so this fix is verified by workflow-level observation rather than unit tests.

It is possible to add a call to config-npm here for validation: https://github.com/SonarSource/sonar-dummy-js/blob/master/.github/workflows/build.yml#L59

[sonar-review-alpha]

Summary

Fixes noisy template parsing errors when config-npm is called a second time in the same job. The issue occurs on the skip path when CONFIG_NPM_COMPLETED is already set: the secrets step is gated out, but GitHub Actions still evaluates the step-level env expression with an empty steps.secrets.outputs.vault, causing fromJSON('') to fail.

The fix makes the expression null-safe by short-circuiting to an empty string if the vault output is missing—a one-line change using the exact pattern already proven stable in config-maven, config-gradle, and get-build-number.

What reviewers should know

What to focus on:

• This is a minimal template expression change in config-npm/action.yml (line 109).
• Verify the null-safe logic: steps.secrets.outputs.vault && short-circuits to || '' when empty.
• Confirm the pattern matches the established convention in peer actions (author references them).

Non-obvious points:

• The skip path (when CONFIG_NPM_COMPLETED is set) is not exercised by the existing spec/config-npm_spec.sh unit tests—only the shell scripts are tested, not the YAML composite action itself.
• This is defensive: the fix prevents error annotations on the second invocation without changing first-invocation behavior.
• GitHub Actions evaluates all step-level env expressions eagerly, even when the step is gated out—hence why the guard condition in if: alone is insufficient.

Test coverage note: The PR relies on workflow-level observation to verify the fix (author's test plan), not automated tests. Check that the merge-queue CI still passes to confirm the first-invocation path is unchanged.

---

• Generate Walkthrough
• Generate Diagram

🗣️ Give feedback

[Copilot]

Pull request overview

This PR updates the config-npm composite GitHub Action to avoid GitHub Actions template parsing errors when the action is invoked more than once in the same job and takes the “skip” path (where the Vault step doesn’t run and produces no outputs).

Changes:

• Make ARTIFACTORY_ACCESS_TOKEN evaluation null-safe by short-circuiting when steps.secrets.outputs.vault is empty, matching the established pattern used in other actions in this repo.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.