Skip to content

feat: onboarding setup-bazel action#1

Open
Raj-StepSecurity wants to merge 3 commits into
mainfrom
release
Open

feat: onboarding setup-bazel action#1
Raj-StepSecurity wants to merge 3 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 Jun 29, 2026
Comment thread config.js
Comment thread config.js

for (const name in externalCacheConfig.manifest) {
externalCache[name] = {
enabled: externalCacheConfig.manifest[name] != false,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use strict equality (!==) instead of loose equality (!=)

Loose equality can cause unexpected type coercion. Use strict equality to match the pattern used elsewhere in this file (e.g., repositoryCacheConfig !== false).

Suggested change
enabled: externalCacheConfig.manifest[name] != false,
enabled: externalCacheConfig.manifest[name] !== false,

Comment thread .github/workflows/ci.yml
- uses: actions/checkout@v7
- uses: ./
- if: failure() && runner.debug == '1'
uses: mxschmitt/action-tmate@v3

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Third-party action not pinned to a commit SHA

mxschmitt/action-tmate is not an official GitHub action and is pinned only to a mutable tag (v3), not to a specific commit SHA. A compromised tag could lead to arbitrary code execution in your runners. Pin to a full SHA:

Suggested change
uses: mxschmitt/action-tmate@v3
uses: mxschmitt/action-tmate@e5c7151931ca95bad1c6f4190c730827f1602a3 # v3

(Verify the SHA against the upstream repo before merging.)

Comment thread .github/workflows/ci.yml
os: ${{ github.actor == 'dependabot[bot]' && fromJSON('["ubuntu"]') || fromJSON('["ubuntu", "macos", "windows"]') }}

steps:
- uses: actions/checkout@v7

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actions/checkout@v7 is not pinned to a commit SHA

Pin to a specific commit SHA to protect against supply-chain attacks via a mutable tag.

Suggested change
- uses: actions/checkout@v7
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4

(Verify the SHA; also note that the latest stable release is v4, not v7.)

@github-actions

Copy link
Copy Markdown

PR Review

Action Type

Node-based — uses node24 runtime with dist/main/index.js and dist/post/index.js as entry points.


✅ Passed Checks

  • License: MIT license present with dual copyright — original author (Alex Rodionov, 2023) and StepSecurity (2026).
  • SECURITY.md: Present with contact email security@stepsecurity.io.
  • FUNDING.yml: Not present (correct).
  • renovate.json: Not present (correct).
  • PULL_REQUEST.md: Not present (correct).
  • CHANGELOG.md: Not present (correct).
  • .vscode folder: Not present (correct).
  • ISSUE_TEMPLATE folder: Not present (correct).
  • Workflow files: .github/workflows/ contains both auto_cherry_pick.yml and actions_release.yml.
  • README banner: StepSecurity Maintained Action banner is present at the top of README.md.
  • README versioning: All usage examples use @v0 (major version only), not a complete semver tag — correct.
  • Subscription check: validateSubscription() is present in index.js and calls the correct URL: https://agent.api.stepsecurity.io/v1/github/${GITHUB_REPOSITORY}/actions/maintained-actions-subscription.
  • Upstream variable: Set to 'bazel-contrib/setup-bazel', which correctly matches original-owner: "bazel-contrib" and repo-name: "setup-bazel" in auto_cherry_pick.yml.
  • package.json author: Set to "step-security".
  • package.json repository field: Not present — ignored per review criteria.
  • action.yml author field: Not present — ignored per review criteria.
  • dist/ folder: Present with both main/ and post/ subdirectories.
  • Build script: package.json has a "build" script using ncc.
  • Subscription only blocks private repos: Public repositories skip the API check — correct.

❌ Failed Checks

  • config.js line 60 — Logic bug in diskCacheName condition: The check if (diskCacheName !== 'true') is always true because diskCacheName is initialized to 'disk' and never set to 'true'. The intent is to check diskCacheConfig !== 'true' (the user input value). As-is, disk-cache: true produces a cache key named disk-true instead of disk. (Inline comment posted.)

  • config.js line 130 — Loose equality (!=) for boolean check: externalCacheConfig.manifest[name] != false uses loose equality, inconsistent with !== false used elsewhere. Can produce unexpected behavior due to type coercion. (Inline comment posted.)

  • ci.yml lines 20 & 23 — Unpinned actions: Both actions/checkout@v7 and mxschmitt/action-tmate@v3 are pinned to mutable version tags rather than immutable commit SHAs. Supply-chain attacks can compromise mutable tags. (Inline comments posted.) Additionally, actions/checkout@v7 does not appear to be a valid published release — the latest stable is v4.


⚠️ Warnings

  • mxschmitt/action-tmate is a third-party remote-debugging action that opens an SSH tunnel to the runner. While it is gated behind failure() && runner.debug == '1', at minimum it must be pinned to a commit SHA.
  • RUNNER_TEMP used for credential temp file (config.js line 83): If RUNNER_TEMP is unset the mkdtempSync call will throw. Consider adding a fallback with os.tmpdir().
  • Cache key injection via disk-cache input (config.js line 61): The raw user-supplied value of disk-cache is embedded in the cache key name without sanitization. Characters like / or spaces could produce unexpected cache key behavior.
  • actions/checkout@v7 — no such release exists; the latest is v4. This may fail at runtime or silently resolve to an unexpected version.

🔒 Security Findings

  1. Unpinned third-party action in CI (.github/workflows/ci.yml line 23): mxschmitt/action-tmate@v3 is not pinned to a commit SHA. A compromised or hijacked tag could execute arbitrary code on the runner with full token access.

  2. Unpinned action with invalid version (.github/workflows/ci.yml line 20): actions/checkout@v7 does not exist as a stable release and is also not SHA-pinned.

  3. Google credentials not masked (config.js lines 84–85): The google-credentials input value is written to disk and embedded in ~/.bazelrc without first calling core.setSecret() to mask it in logs. This could expose the key in debug output.

  4. Google credentials written to persistent file: The path to the credentials temp file is embedded in ~/.bazelrc (which persists for the runner's lifetime). Although the file is deleted in the post step, the bazelrc path reference remains after deletion.


Summary

The onboarding is largely complete and follows StepSecurity conventions — license, banner, subscription check, and required workflow files are all present and correctly configured. The main blockers are a logic bug in disk cache key naming (diskCacheName vs diskCacheConfig), unpinned/invalid actions in the CI workflow, and missing core.setSecret() masking for the Google credentials input.

subscription checks added and other changes

subscription checks added and other changes
Comment thread index.js
const fileContent = fs.readFileSync(filePath)
const actualHash = crypto.createHash('sha256').update(fileContent).digest('hex')
if (actualHash !== expectedHash) {
core.warning(`Checksum mismatch for Bazelisk. Expected ${expectedHash}, got ${actualHash}`)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security: checksum mismatch should fail, not warn.

core.warning() here logs a message but allows execution to continue — a tampered Bazelisk binary will still be installed and run on the runner.

Suggested change
core.warning(`Checksum mismatch for Bazelisk. Expected ${expectedHash}, got ${actualHash}`)
throw new Error(`Checksum mismatch for Bazelisk. Expected ${expectedHash}, got ${actualHash}`)

This should be a hard failure to prevent supply-chain attacks via a modified binary.

Comment thread .github/workflows/ci.yml
os: ${{ github.actor == 'dependabot[bot]' && fromJSON('["ubuntu"]') || fromJSON('["ubuntu", "macos", "windows"]') }}

steps:
- uses: actions/checkout@v7

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actions/checkout@v7 does not exist.

The latest stable release is v4. This will cause the CI workflow to fail with a resolution error.

Suggested change
- uses: actions/checkout@v7
- uses: actions/checkout@v4

Comment thread config.js
// with Bazel 6.3 and onwards, common accepts any legal Bazel option
// https://github.com/bazelbuild/bazel/issues/3054
bazelrc.push(`build --disk_cache=${bazelDisk}`)
if (diskCacheName !== 'true') {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logic bug: condition is always true.

diskCacheName is hardcoded to 'disk' two lines above, so diskCacheName !== 'true' is always true. The intended guard is against the user setting disk-cache: true (i.e. the raw input), to avoid producing a cache key like disk-true.

Suggested change
if (diskCacheName !== 'true') {
if (diskCacheConfig !== 'true') {

@github-actions

Copy link
Copy Markdown

PR Review

Action Type

Node-based action (runtime: node24, entry points: dist/main/index.js and dist/post/index.js).


✅ Passed Checks

  • License: Present with dual copyright — original author (Alex Rodionov, 2023) and StepSecurity (2026).
  • action.yml: Present (no author field, ignored per policy).
  • SECURITY.md: Present with correct reporting email.
  • No FUNDING.yml: Not present.
  • auto_cherry_pick.yml: Present in .github/workflows/.
  • actions_release.yml: Present in .github/workflows/.
  • 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 usage examples: All examples correctly use only the major version tag (step-security/setup-bazel@v0).
  • StepSecurity Maintained Action banner: Present at the top of README.md.
  • Subscription check URL: Correct — calls https://agent.api.stepsecurity.io/v1/github/${GITHUB_REPOSITORY}/actions/maintained-actions-subscription.
  • Upstream variable value: bazel-contrib/setup-bazel — correctly matches original-owner: "bazel-contrib" and repo-name: "setup-bazel" from auto_cherry_pick.yml.
  • package.json author: Set to step-security.
  • No unused dependencies: All 7 declared dependencies (axios, @actions/cache, @actions/core, @actions/github, @actions/glob, @actions/tool-cache, yaml) are actively used.
  • dist folder: Present with both dist/main/ and dist/post/ sub-directories.
  • build script in package.json: Present (ncc build index.js -s -o dist/main && ncc build post.js -s -o dist/post).

❌ Failed Checks

  • Security: checksum mismatch does not fail the actionverifyChecksum() in index.js (line 92) calls core.warning() when the downloaded Bazelisk binary's SHA-256 hash does not match the expected value, then continues execution. A supply-chain attacker who replaces the binary would pass undetected. This must throw an error or call core.setFailed() to halt execution.

  • ci.yml references a non-existent action version — Line 20 uses actions/checkout@v7, which does not appear to exist (latest stable release is v4). This will likely cause the CI workflow to fail with a resolution error. This is likely a typo; it should be actions/checkout@v4.


⚠️ Warnings

  • Logic bug in config.js (line 60): The condition if (diskCacheName !== 'true') always evaluates to true because diskCacheName is hardcoded to 'disk' two lines earlier. The intended check is almost certainly if (diskCacheConfig !== 'true'), to avoid appending the literal string true to the cache key name when disk-cache: true is set.

  • BAZELISK_GITHUB_TOKEN exported via core.exportVariable() (config.js line 154): This makes the GitHub token visible to all subsequent steps in the workflow job via process.env. Consider using core.setSecret() in addition to mask the value in logs.

  • ci.yml uses mxschmitt/action-tmate@v3 without SHA pinning (line 22): Third-party actions used in CI workflows should be pinned to a full commit SHA rather than a mutable tag to prevent tag-squatting attacks.

  • No tests defined: package.json has "test": "echo \"Error: no test specified\" && exit 1". Even basic smoke tests or input validation tests would improve confidence during the release workflow.


🔒 Security Findings

  1. [Critical] Checksum mismatch does not halt execution (index.js line 92): verifyChecksum() warns but never throws. A compromised Bazelisk binary served from GitHub Releases would be silently installed onto the runner. Fix: replace core.warning() with throw new Error().

  2. [Moderate] GitHub token leaked to environment (config.js line 154): core.exportVariable('BAZELISK_GITHUB_TOKEN', token) exports the token to the OS environment, where it is readable by any subsequent step or child process in the job. Consider masking it with core.setSecret(token) before exporting.


Summary

The onboarding is well-structured and passes most required checks. Two issues must be resolved before merge: a critical security flaw where a checksum mismatch only warns instead of failing (allowing a tampered Bazelisk binary to run), and a broken CI workflow referencing the non-existent actions/checkout@v7. The logic bug in config.js around diskCacheName should also be corrected.

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.

1 participant