[aatarasoff]

Summary

• Upgrades @actions/core (^1.11.1), @babel/core, @babel/preset-env, @babel/eslint-parser, eslint, eslint-plugin-jest, and prettier (superseding Bump @babel/preset-env from 7.23.3 to 7.23.9 #18, Bump @babel/core from 7.23.3 to 7.23.9 #19, Bump prettier from 3.1.0 to 3.2.5 #20, Bump eslint-plugin-jest from 27.6.0 to 27.9.0 #21, Bump eslint from 8.54.0 to 8.57.0 #23).
• Resolves all 20 fixable npm audit / Dependabot advisories (uuid, js-yaml, brace-expansion, braces, browserslist, cross-spawn, flatted, minimatch, picomatch, @babel/*), leaving 0 production vulnerabilities (npm audit --omit=dev) and shrinking dist/index.js from 151 kB to 140 kB.

[aatarasoff]

Code Review: Security & Reliability Focus

Context: Node.js GitHub Action for installing pbuf CLI (via remote script) and optionally running schema drift checks with sticky PR comments. High-concurrency CI environments, public-facing in the sense of running on arbitrary repos with user-provided tokens/versions.

Findings

• [Severity: High] — Security & Vulnerabilities

  • Location: src/main.js — run() function, the exec.exec('/bin/bash', ['-lc', curl -fsSL ${INSTALL_SCRIPT_URL} | sh ...]) blocks (approx. lines for version install).
  • Problem: The action downloads and pipes a remote install script (https://raw.githubusercontent.com/pbufio/pbuf-registry/main/scripts/install.sh) directly into sh without any integrity verification (checksum, signature, or commit pin). If the GitHub repo or the main branch is compromised (or a malicious PR is merged), every workflow using this action becomes a supply-chain attack vector. The version is passed safely as a positional arg for non-latest cases (good), but the script itself is unsigned. This is a classic curl | sh risk in CI.
  • Fix: Prefer downloading a pinned release binary directly from GitHub Releases (with optional SHA256 verification), or pin the install script to a specific commit SHA and verify a checksum. Example sketch:

    // Prefer direct binary download + checksum for known versions
    const RELEASE_URL = `https://github.com/pbufio/pbuf-registry/releases/download/${version}/pbuf_${version.slice(1)}_linux_amd64.tar.gz`;
    // Download, verify sha256 against a known manifest, extract, and chmod +x.
    // Fall back to script only with explicit user opt-in and clear warning.

• [Severity: Medium] — Security & Vulnerabilities

  • Location: src/main.js — run() → exec.exec('pbuf', ['auth', pbufToken]).
  • Problem: The registry token is passed as a command-line argument. On Linux this is visible in /proc/<pid>/cmdline and process listings (ps, audit logs, container introspection). Combined with core.setSecret (which only masks logs), it still leaks to the process table for the duration of the auth call.
  • Fix: Prefer an environment variable or stdin if the CLI supports it. If not:

    await exec.exec('pbuf', ['auth'], {
      env: { ...process.env, PBUF_TOKEN: pbufToken }
    });
    // Or document that the CLI must be updated to read from env/stdin.

    Ensure the CLI reads from PBUF_TOKEN or a file.

• [Severity: Medium] — Correctness & Edge Cases / Architecture

  • Location: PR title/body vs. action.yml.
  • Problem: Title and summary claim "default CLI to v0.17.1", but action.yml still defaults to 'v0.17.0' and no source change updates it. Current registry releases appear to be around v0.16.x. This creates a documentation/code mismatch and may confuse consumers.
  • Fix: Either update action.yml default (and README) to the intended version after confirming it exists and is stable, or correct the PR title/body to reflect only the npm dependency upgrades.

Positive Notes

• Version regex (VERSION_RE) properly restricts input before shell use.
• crypto.randomUUID() replacement for the old uuid package in @actions/core is a solid security improvement (avoids the old uuid advisory).
• Proper secret masking, input validation for fail_on, repository regex, fetch timeouts, and try/catch around the main path.
• Bundle size reduction and zero production vulns are good outcomes.

Go / No-Go Verdict

Conditional Go. The dependency upgrades themselves look clean and the rebuilt dist/index.js is consistent with the new @actions/core. No new critical regressions were introduced in the source (which is unchanged). However, the pre-existing curl | sh supply-chain risk is High severity for a setup action used in CI and should be addressed before broad reliance, or at least clearly documented with a mitigation path. The title/default mismatch should be fixed for accuracy. Safe to merge for the security dep bumps once the documentation inconsistency is corrected; schedule a follow-up for the install method.

Review performed as Principal Software Engineer / Security Researcher. Source logic reviewed; dist changes are the expected ncc rebuild.