Skip to content

feat: onboarding setup-helm action - #1

Merged
Raj-StepSecurity merged 4 commits into
mainfrom
release
Jul 31, 2026
Merged

feat: onboarding setup-helm action#1
Raj-StepSecurity merged 4 commits into
mainfrom
release

Conversation

@Raj-StepSecurity

Copy link
Copy Markdown
Collaborator

No description provided.

@Raj-StepSecurity Raj-StepSecurity added the review-required Request Claude AI code review on the PR label Jul 31, 2026
@github-actions

Copy link
Copy Markdown

PR Review

Action Type

Node-based — TypeScript source compiled to dist/index.js, executed by the node24 runner runtime.


✅ Passed Checks

  • License: Present with dual copyright — Microsoft (original) and StepSecurity (2026). ✅
  • action.yml: Present and well-formed. No author field present (ignored per policy). ✅
  • SECURITY.md: Present with correct contact email. ✅
  • No FUNDING.yml: Not present. ✅
  • Workflows: .github/workflows/ contains both auto_cherry_pick.yml and actions_release.yml. ✅
  • No renovate.json: Not present. ✅
  • No PULL_REQUEST.md: Not present. ✅
  • No ISSUE_TEMPLATE: Folder not present. ✅
  • No CHANGELOG.md: Not present. ✅
  • No .vscode: Folder not present. ✅
  • README versioning: Example uses step-security/setup-helm@v5 (major version only, not a full semver tag). ✅
  • README banner: StepSecurity maintained action banner is present and correctly linked. ✅
  • Subscription check URL: Calls the correct endpoint https://agent.api.stepsecurity.io/v1/github/${GITHUB_REPOSITORY}/actions/maintained-actions-subscription. ✅
  • upstream variable: Set to azure/setup-helm, correctly matching original-owner: "azure" + repo-name: "setup-helm" in auto_cherry_pick.yml. ✅
  • package.json author: Set to step-security. ✅
  • No repository field in package.json: Not present (ignored per policy). ✅
  • dist/ folder: Present. ✅
  • build script: Present in package.json scripts. ✅
  • Dependencies: All listed dependencies (@actions/core, @actions/tool-cache, axios) are actively used. ✅

❌ Failed Checks

None.


⚠️ Warnings

  • audit_package.yml permissions misplaced: The permissions block appears after the jobs block. YAML maps are order-independent so GitHub Actions accepts it, but convention is to place permissions before jobs.
  • Untyped parameters in walkSync (src/run.ts:442): The dir, filelist, and fileToFind parameters have no TypeScript type annotations in an otherwise fully-typed file. Should be typed as string, string[], and string respectively.
  • Workflow actions not SHA-pinned: unit-tests.yml and integration-tests.yml use actions/checkout@v7 with a mutable tag. Best practice is to pin to a full commit SHA.
  • process.exit(1) in validateSubscription() (src/run.ts:57): Hard-exiting bypasses cleanup logic and prevents core.setFailed() from recording a proper failure. Prefer throw new Error(...) so the run().catch(core.setFailed) handler in index.ts reports the failure cleanly.

🔒 Security Findings

[HIGH] SHA256 checksum mismatch is non-fatalsrc/run.ts:363

In verifyHelmSHA256(), when the computed SHA256 does not match the expected value, only a core.warning() is emitted — the function returns normally and the tampered binary is silently installed. This defeats the purpose of integrity verification and is a supply-chain risk. The function must throw on mismatch so the download chain fails:

if (actualHash !== expectedHash) {
  throw new Error(
    `SHA256 mismatch. Expected: ${expectedHash}, Got: ${actualHash}`
  )
}

Summary

The onboarding passes all structural and policy checks. One high-severity security issue must be addressed before merge: the SHA256 integrity check silently accepts a tampered binary on mismatch. The untyped walkSync parameters and the process.exit(1) pattern are lower-priority but should also be resolved.

Comment thread src/run.ts
Comment thread src/run.ts
Comment thread package.json Outdated
amanstep
amanstep previously approved these changes Jul 31, 2026
@github-actions

Copy link
Copy Markdown

PR Review

Action Type

Node-based — uses node24 runtime as defined in action.yml.


✅ Passed Checks

  • License: Present with both Microsoft (original) and StepSecurity copyright.
  • SECURITY.md: Present at repo root.
  • No FUNDING.yml: Not present (root or .github/).
  • No renovate.json: Not present.
  • No PULL_REQUEST.md: Not present.
  • No ISSUE_TEMPLATE folder: Not present.
  • No CHANGELOG.md: Not present.
  • No .vscode folder: Not present.
  • Workflow: actions_release.yml: Present in .github/workflows/.
  • Workflow: auto_cherry_pick.yml: Present in .github/workflows/.
  • README banner: StepSecurity maintained action banner is present.
  • README examples use major version only: Both examples correctly use step-security/setup-helm@v5 (not a full semver tag).
  • Subscription check URL: Correctly calls https://agent.api.stepsecurity.io/v1/github/${GITHUB_REPOSITORY}/actions/maintained-actions-subscription.
  • Upstream variable matches cherry-pick config: upstream = 'azure/setup-helm' in run.ts matches original-owner: "azure" / repo-name: "setup-helm" in auto_cherry_pick.yml.
  • package.json author: Set to step-security.
  • No unused dependencies: All three runtime deps (@actions/core, @actions/tool-cache, axios) are actively used.
  • dist/ folder: Present and contains index.js.
  • Build script present: package.json contains a "build" script, so no script input is required in release workflows.

❌ Failed Checks

  • SHA256 mismatch does not fail the actionverifyHelmSHA256 in src/run.ts (line 364) only calls core.warning() on a checksum mismatch instead of throwing an error. A tampered or MITM-swapped Helm binary will be silently installed. This must throw an error to abort the step.

  • process.exit(1) used instead of core.setFailed()src/run.ts line 56 calls process.exit(1) inside validateSubscription(). This hard-kills the runner process and prevents any GitHub Actions post-job cleanup steps from executing. The correct pattern is core.setFailed(...) followed by return.


⚠️ Warnings

  • Workflow actions not SHA-pinned: actions/checkout@v7 is used across integration-tests.yml, unit-tests.yml, and prettify-code.yml without a commit SHA pin. Best practice for supply-chain security is to pin to a full SHA.
  • downloadBaseURL input is unvalidated: The action accepts an arbitrary URL with no domain allowlist. While configured by workflow authors, this is dangerous given the SHA256 check is non-blocking (see security findings).

🔒 Security Findings

  1. [High] Silent SHA256 mismatchsrc/run.ts:364: verifyHelmSHA256 calls core.warning() on a hash mismatch, allowing a tampered binary to be installed without blocking the workflow. Combined with the unvalidated downloadBaseURL input, this creates a path for serving a malicious Helm binary undetected. The function must throw (or call core.setFailed) to abort on mismatch.

  2. [Medium] process.exit(1) bypasses cleanupsrc/run.ts:56: Directly calling process.exit(1) skips all GitHub Actions post-job hooks and cleanup steps. Use core.setFailed(message); return instead.


Summary

The onboarding PR meets most structural and policy requirements. Two issues must be fixed before merging: verifyHelmSHA256 must throw on a checksum mismatch (not just warn), and process.exit(1) must be replaced with core.setFailed() + return to preserve the GitHub Actions lifecycle.

Comment thread src/run.ts
Comment thread src/run.ts
@Raj-StepSecurity
Raj-StepSecurity merged commit 8a20648 into main Jul 31, 2026
14 checks passed
@Raj-StepSecurity
Raj-StepSecurity deleted the release branch July 31, 2026 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-required Request Claude AI code review on the PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants