Skip to content

feat: Onboard benchmark-action/github-action-benchmark action - #1

Merged
anurag-stepsecurity merged 2 commits into
mainfrom
release
Jul 14, 2026
Merged

anurag-stepsecurity merged 2 commits into
mainfrom
release

Conversation

@anurag-stepsecurity

Copy link
Copy Markdown
Collaborator

@anurag-stepsecurity
anurag-stepsecurity force-pushed the release branch 4 times, most recently from 4437db5 to a87fc8a Compare July 14, 2026 08:03
@anurag-stepsecurity anurag-stepsecurity added the review-required Request Claude AI code review on the PR label Jul 14, 2026
Comment thread src/git.ts
Comment thread src/config.ts
@github-actions

Copy link
Copy Markdown
Contributor

placeholder review comment - will be replaced

@github-actions

Copy link
Copy Markdown
Contributor

PR Review\n\n### Action Type\nNode-based action - runs on node24, entry point at dist/src/index.js.\n\n### Passed Checks\n- License: Present with dual copyright (StepSecurity 2026 and original author rhysd 2019)\n- action.yml: Present with author step-security\n- SECURITY.md: Present\n- FUNDING.yml: Not present\n- auto_cherry_pick.yml and actions_release.yml: Both present\n- renovate.json: Not present\n- PULL_REQUEST.md: Not present\n- ISSUE_TEMPLATE folder: Not present\n- CHANGELOG.md: Not present\n- .vscode folder: Not present\n- README.md banner: StepSecurity banner present at top\n- README.md semver: All examples use @v1 major-version-only tags\n- Subscription check URL: Correct endpoint used\n- Upstream variable: benchmark-action/github-action-benchmark matches auto_cherry_pick.yml\n- package.json author: step-security\n- package.json repository: Contains step-security\n- dist folder: Present at dist/src/index.js\n- Build script: Present in package.json\n- Dependencies: All runtime deps (axios, zod, @actions/*) actively used\n\n### Failed Checks\nNone.\n\n### Warnings\n- alert-comment-cc-users validation is too permissive: only checks for @ prefix, no newline/special-char restriction - markdown injection possible (inline comment on src/config.ts)\n- Token embedded in raw Error.message in src/git.ts: core.setSecret() masks it in Actions logs but Error.message is unsanitized (inline comment on src/git.ts)\n- git push --no-verify: pre-push hooks skipped, intentional but worth documenting\n\n### Security Findings\n1. Token leak risk in error messages (src/git.ts:71): cmd() joins git args including the token-bearing remote URL into error message strings. core.setSecret() masks it in Actions log sinks but not in raw Error.message. Inline suggestion posted.\n2. Markdown injection via alert-comment-cc-users (src/config.ts:169-174): Only @ prefix check enforced; newlines allow injecting arbitrary markdown into comments. Inline suggestion posted.\n\n### Summary\nOnboarding is structurally complete - all required files present, subscription check correctly wired, upstream attribution matches, README and action metadata reference step-security correctly. Two security hardening items (token redaction in error messages and stricter username validation) should be addressed before merging.

Signed-off-by: Anurag Rajawat <anurag@stepsecurity.io>
@github-actions

Copy link
Copy Markdown
Contributor

PR Review

Action Type

Node-based action — uses node24 runtime in action.yml, built with @vercel/ncc from TypeScript sources.


Passed Checks

  • License: MIT license present with copyright for both StepSecurity (2026) and original author (rhysd, 2019).
  • action.yml: Present with author: 'step-security' correctly set.
  • SECURITY.md: Present with correct reporting email (security@stepsecurity.io).
  • No FUNDING.yml: File is absent as required.
  • Workflows present: .github/workflows/ contains both auto_cherry_pick.yml and actions_release.yml.
  • No renovate.json: File is absent as required.
  • No PULL_REQUEST.md: File is absent as required.
  • No ISSUE_TEMPLATE folder: Absent as required.
  • No CHANGELOG.md: Absent as required.
  • No .vscode folder: Absent as required.
  • README semver: All references to step-security/github-action-benchmark use only the major version tag @v1 — no full semver (e.g. v1.2.3) found.
  • README banner: StepSecurity maintained action banner is present at the top of README.md.
  • Subscription check: validateSubscription() is present in src/subscription.ts and correctly calls the maintained-actions-subscription endpoint.
  • Upstream variable: upstream = 'benchmark-action/github-action-benchmark' in subscription.ts correctly matches original-owner: 'benchmark-action' / repo-name: 'github-action-benchmark' from auto_cherry_pick.yml.
  • package.json author: Set to "step-security".
  • package.json repository: URL contains step-security.
  • dist folder: Present with compiled dist/src/index.js.
  • Build script: "build": "ncc build src/index.ts -o dist/src" is present in package.json — no script input needed in release/audit workflows.

Failed Checks

No hard-failing checklist items were found.


Warnings

  • Personal username in example workflows: Multiple workflow files (benchmarkdotnet.yml, benchmarkjs.yml, commit-comment.yml, cpp.yml, go.yml, etc.) hardcode alert-comment-cc-users: '@anurag'. This is a personal GitHub handle; consider replacing with a team mention or removing the field from example workflows entirely.

  • ci.yml lacks a top-level permissions block: The CI workflow has no workflow-level permissions: declaration, so the GITHUB_TOKEN inherits whatever the repository default allows. Add permissions: contents: read (or narrower per-job) as a best practice.

  • process.exit(1) in subscription check: src/subscription.ts calls process.exit(1) on a 403 response instead of the conventional core.setFailed() approach. process.exit skips any finally blocks and pending async cleanup in the action runtime.

  • @types/rimraf version mismatch: devDependencies declares @types/rimraf@^2.0.3 against rimraf@^3.0.2. These are incompatible major versions; update to @types/rimraf@3.x.

  • CI workflow not using step-security/harden-runner: None of the jobs in ci.yml include step-security/harden-runner, which is expected for a StepSecurity-maintained action repo.


Security Findings

  • Token exposure in git error messages (low severity, mitigated): In src/git.ts, the capture() helper builds error messages from args.join(' '), which can include the https://x-access-token:${token}@... remote URL on git failure. This is mitigated by core.setSecret(config.githubToken) called in index.ts before any git operations, which causes the Actions runner to redact the token from all log output. Consider also redacting the URL in the thrown Error message as defence-in-depth.

  • No other exploitable security vulnerabilities were identified. Input validation for paths, thresholds, CC users, and gh-repository is present and appears adequate.


Summary

The PR successfully onboards the upstream benchmark-action/github-action-benchmark action into the StepSecurity ecosystem and passes all mandatory checklist items. The main items to address before merge are replacing or removing the hardcoded @anurag personal username from example workflows and adding an explicit permissions block to ci.yml.

Comment thread src/subscription.ts
Comment thread examples/workflows/benchmarkdotnet.yml
Comment thread src/git.ts
Comment thread .github/workflows/ci.yml
Comment thread package.json
Comment thread examples/workflows/benchmarkdotnet.yml
Comment thread examples/workflows/benchmarkjs.yml
Comment thread .github/workflows/commit-comment.yml
Comment thread images/alert-comment.png
Comment thread .yamllint.yml
Comment thread LICENSE
@anurag-stepsecurity anurag-stepsecurity removed the review-required Request Claude AI code review on the PR label Jul 14, 2026
@anurag-stepsecurity
anurag-stepsecurity merged commit 4dd87db into main Jul 14, 2026
19 of 20 checks passed
@anurag-stepsecurity
anurag-stepsecurity deleted the release branch July 14, 2026 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants