Skip to content

feat: Onboard actions4git/add-commit-push action#1

Merged
anurag-stepsecurity merged 1 commit into
mainfrom
release
Jul 17, 2026
Merged

feat: Onboard actions4git/add-commit-push action#1
anurag-stepsecurity merged 1 commit into
mainfrom
release

Conversation

@anurag-stepsecurity

Copy link
Copy Markdown
Collaborator

@anurag-stepsecurity anurag-stepsecurity added the review-required Request Claude AI code review on the PR label Jul 16, 2026
Comment thread main.sh
Comment thread main.sh
Comment thread _main.mjs
Comment thread .github/workflows/test-action.yml
Comment thread .github/workflows/test-action.yml
Comment thread package.json
Comment thread main.sh
Comment thread package.json
@github-actions

Copy link
Copy Markdown

PR Review

Action Type

Node-based action — uses runs: { using: node24, main: dist/index.mjs }. The main logic is a Bash script (main.sh) spawned from a Node.js entry point (_main.mjs) after the subscription check runs.


✅ Passed Checks

  • License present with dual copyright: original author (GitHub Actions for Git, 2023) and StepSecurity (2026)
  • action.yml is present (no author field, which is acceptable per review criteria)
  • SECURITY.md is present
  • FUNDING.yml / funding.yml absent
  • .github/workflows/auto_cherry_pick.yml present
  • .github/workflows/actions_release.yml present
  • renovate.json absent
  • PULL_REQUEST.md absent
  • CHANGELOG.md absent
  • ISSUE_TEMPLATE folder absent
  • .vscode folder absent
  • README.md contains the StepSecurity maintained-action banner
  • README usage examples use only the major version tag (@v1) — no complete semver tags
  • Subscription check is present in subscription.mjs and calls the correct URL: https://agent.api.stepsecurity.io/v1/github/${GITHUB_REPOSITORY}/actions/maintained-actions-subscription
  • Subscription upstream value (actions4git/add-commit-push) correctly matches original-owner/repo-name in auto_cherry_pick.yml
  • dist/ folder is present
  • package.json contains a build script
  • actions_release.yml has a node_version input wiring — build is handled via the reusable workflow

❌ Failed Checks

  • package.json missing author field — should be set to step-security per the node-action checklist.
  • package.json missing repository field — should reference the step-security fork URL.
  • actions/checkout@v7 in test-action.yml is unpinned — tag references are mutable and should be pinned to a full commit SHA for supply-chain security. Note: v7 does not currently exist for actions/checkout; latest is v4.
  • Bug in main.sh lines 62–63 — the commit_author name assignment is immediately overwritten by the email assignment. The variable ends up holding only the raw email string, which the downstream sed parsing cannot split into name/email correctly. The git author name will be set to the email string instead of github-actions[bot]. Same bug exists on lines 67–68 for the me/author/self special value. See inline comments for suggested fixes.
  • push-branch used in test-action.yml but not defined in action.ymlaction.yml defines push-refspec as the canonical input; push-branch is only an undocumented alias in main.sh. The test workflow should use push-refspec instead.
  • Left-over development artifact in _main.mjs — the comment // 👈 CHANGE ME! on line 8 is a placeholder that was not removed before submission.

⚠️ Warnings

  • @actions/github listed as a dependency but appears unused — neither subscription.mjs nor _main.mjs import it. Verify whether it is actually needed; if not, remove it to minimise the dependency surface.
  • $GITHUB_OUTPUT is unquoted in all append statements across main.sh (e.g., >> $GITHUB_OUTPUT). Should be >> "$GITHUB_OUTPUT" for robustness on self-hosted runners where the path might contain spaces.
  • Reusable workflows pinned by tag (@v1) not SHAstep-security/reusable-workflows references in all three workflow files use mutable version tags rather than commit SHAs.
  • dist/main.sh is a verbatim copy of main.sh — having two identical copies in the repo is fragile; changes to main.sh must also be reflected in dist/main.sh. The build script already copies it, so the copy committed to dist/ will drift if main.sh is ever edited without re-running the build.

🔒 Security Findings

  • Unquoted flag variables in shell commands$commit_date_flag and $add_force_flag are unquoted in git invocations (e.g., git commit ... $commit_date_flag). Since these variables are strictly controlled (either empty or a known flag string), the immediate risk is low, but quoting them is best practice to prevent unintended word-splitting.
  • commit_author bug (also a security consideration) — Because of the overwrite bug on lines 62–63 / 67–68, a user who specifies commit-author: github-actions will get a git commit whose author name is set to an email address string. This is inconsistent with the documented behavior and could mislead tooling that inspects commit metadata.
  • No other exploitable injection vectors were found. Input values are passed through environment variables and quoted in shell expansions; GITHUB_OUTPUT writes use the safe key=value append form; path traversal in the path input is guarded by the realpath workspace boundary check.

Summary

The onboarding structure is sound — subscription check, licensing, workflows, and dist are all in place. However there are two logic bugs in the special-value handling of commit-author in main.sh that will produce incorrect git author metadata, plus several minor issues (undocumented input alias in the test, leftover development comment, missing package.json metadata fields) that should be resolved before merging.

@github-actions

Copy link
Copy Markdown

PR Review

Action Type

Node-based action — uses node24 runtime via dist/index.mjs (bundled with @vercel/ncc). Also includes a shell script (main.sh) spawned as a subprocess.


✅ Passed Checks

  • License present with dual copyright: GitHub Actions for Git (2023) and StepSecurity (2026)
  • action.yml is present (no author field to check)
  • SECURITY.md is present
  • FUNDING.yml is absent
  • .github/workflows/auto_cherry_pick.yml is present
  • .github/workflows/actions_release.yml is present
  • renovate.json is absent
  • PULL_REQUEST.md is absent
  • ISSUE_TEMPLATE folder is absent
  • CHANGELOG.md is absent
  • .vscode folder is absent
  • README banner — StepSecurity maintained-action banner is present on line 1
  • dist/ folder is present (dist/index.mjs, dist/main.sh)
  • Subscription check URL — correctly calls https://agent.api.stepsecurity.io/v1/github/${GITHUB_REPOSITORY}/actions/maintained-actions-subscription
  • upstream value in subscription.mjs (actions4git/add-commit-push) correctly matches original-owner + repo-name from auto_cherry_pick.yml
  • package.json build script is present (ncc build _main.mjs -o dist && cp main.sh dist/)
  • package.json has no author or repository field (checks waived per review policy)
  • README examples use major-version-only tags (@v1, @v7) — no full semver used

❌ Failed Checks

  • _main.mjs line 8 — placeholder comment not removed: The line still contains // 👈 CHANGE ME!, a leftover template artifact. This must be removed before merge.

  • actions/checkout@v7 referenced everywhere — version does not exist: test-action.yml line 23 and multiple places in README.md reference actions/checkout@v7, but the latest released version is v4. This will cause all test workflow runs to fail with "Unable to resolve action". Must be corrected to actions/checkout@v4.

  • Path traversal vulnerability in main.sh line 8: The workspace boundary check [[ "$resolved_path" != "$resolved_workspace"* ]] has a classic prefix-match bug. If $resolved_workspace is /home/runner/work/repo, a path like /home/runner/work/repo-evil would pass the check. The fix is to append a trailing slash: "$resolved_workspace/"*. Same issue in dist/main.sh.

  • commit_author variable overwrite bug in main.sh lines 62–63 and 67–68: When the special alias github-actions is detected, the code first sets commit_author='github-actions[bot]' (the name), then immediately overwrites it with commit_author='github-actions[bot]@users.noreply.github.com' (the email). The downstream sed parsing that extracts Name <email> then receives a bare email string and silently produces wrong values for both name and email. The same bug exists for the me/self alias. Same bug in dist/main.sh. This is a regression from upstream.


⚠️ Warnings

  • audit_package.ymlpermissions block placed after jobs block (lines 29–33): While YAML mappings are unordered and GitHub Actions will still read it correctly, placing permissions at the end of the file after jobs is unconventional, confusing to reviewers, and grants broad write permissions (contents: write, pull-requests: write, issues: write) at the workflow level without job-level overrides. Recommend moving permissions: {} to the top level and using job-level permissions instead.

  • push-branch undeclared in action.yml: The test workflow (test-action.yml line 27) passes push-branch: test-branch as an input, but action.yml only declares push-refspec. The shell script does accept INPUT_PUSH-BRANCH as a fallback alias, but this undeclared input is invisible to action users and tooling.

  • Empty descriptions for commit-author and commit-committer in action.yml (lines 29, 39): Both are set to description: "". These inputs are non-trivial and their descriptions in the README explain important behavior — the action.yml should mirror them for discoverability.

  • Unquoted $GITHUB_OUTPUT in main.sh (lines 50, 165, 166): >> $GITHUB_OUTPUT should be >> "$GITHUB_OUTPUT" for shell best practice.

  • Subscription check exfiltrates repository identity: Every time this action runs on a private repo, it sends GITHUB_REPOSITORY and GITHUB_ACTION_REPOSITORY to agent.api.stepsecurity.io. This is expected behavior for a maintained action, but it should be clearly documented in the README so users are aware.


🔒 Security Findings

  1. [HIGH] Path traversal in main.sh line 8: String prefix matching without a trailing slash separator allows a directory named <workspace>-evil to bypass the workspace confinement check. Fix: change "$resolved_workspace"* to "$resolved_workspace/"*. Also fix in dist/main.sh.

  2. [MEDIUM] Author/committer identity corruption (main.sh lines 62–68): The double-assignment bug means that when a user passes commit-author: github-actions or commit-author: me, the git commit will be made with an incorrect author identity (the email string gets used as the name, and the sed extraction produces garbage). This could cause commits to be attributed to unexpected identities. Fix: use separate variables for name and email instead of overwriting the combined commit_author variable.

  3. [LOW] actions/checkout@v7 does not exist: While primarily a correctness issue, referencing a non-existent action tag means the action silently resolves to an unpredictable commit (or fails outright), undermining supply-chain integrity. All references should be updated to the pinned actions/checkout@v4.


Summary

The onboarding structure is solid — subscription gating, dual copyright, and the required workflow files are all present. However, this PR has two code-correctness bugs (path traversal and author variable overwrite) that rise to security concern level, plus a checkout@v7 reference that will cause test failures on merge. These must be fixed before merging.

Comment thread _main.mjs
Comment thread .github/workflows/test-action.yml
Comment thread main.sh
Comment thread main.sh
Comment thread main.sh
Comment thread .github/workflows/audit_package.yml
Comment thread action.yml
Comment thread action.yml
Comment thread README.md Outdated
Signed-off-by: Anurag Rajawat <anurag@stepsecurity.io>
Comment thread _main.mjs
Comment thread main.sh
Comment thread main.sh
@github-actions

Copy link
Copy Markdown

PR Review

Action Type

Node-based action (runtime: node24, entry point: dist/index.mjs).


Passed Checks

  • License - MIT license present with dual copyright: original author (GitHub Actions for Git, 2023) and StepSecurity (2026).
  • SECURITY.md - Present with correct reporting contact (security@stepsecurity.io).
  • FUNDING.yml - Not present (correct).
  • renovate.json - Not present (correct).
  • PULL_REQUEST.md - Not present (correct).
  • ISSUE_TEMPLATE folder - Not present (correct).
  • CHANGELOG.md - Not present (correct).
  • .vscode folder - Not present (correct).
  • action.yml - Present; no author field (ignored per criteria).
  • .github/workflows/actions_release.yml - Present.
  • .github/workflows/auto_cherry_pick.yml - Present with correct original-owner: "actions4git" and repo-name: "add-commit-push".
  • README.md banner - StepSecurity maintained action banner is present at the top of the file.
  • README.md version tags - All usage examples reference step-security/add-commit-push@v1 (major version only, no full semver tag).
  • Subscription check URL - Correct: https://agent.api.stepsecurity.io/v1/github/${GITHUB_REPOSITORY}/actions/maintained-actions-subscription.
  • Upstream variable - upstream = 'actions4git/add-commit-push' in subscription.mjs correctly matches <original-owner>/<repo-name> from auto_cherry_pick.yml.
  • dist/ folder - Present (dist/index.mjs and dist/main.sh).
  • package.json build script - Present: ncc build _main.mjs -o dist && cp main.sh dist/.
  • Dependencies - All three dependencies (@actions/core, axios, @vercel/ncc) are actively used.
  • package.json author/repository fields - Not present (ignored per criteria).

Failed Checks

None.


Warnings

  • Template artifact in _main.mjs line 8 - The comment CHANGE ME is a leftover from the onboarding template and was not removed. See inline comment.
  • Test workflow uses undocumented input alias - .github/workflows/test-action.yml passes push-branch: test-branch, but the documented action.yml input is push-refspec. The alias works via the env-var fallback in main.sh, but using the canonical input name in tests would be clearer.
  • audit_package.yml permissions block placement - The top-level permissions block is placed after the jobs block. While technically valid, the conventional position is before jobs for readability.

Security Findings

Bug (carried over from upstream actions4git/add-commit-push): In main.sh, the special-value handling for commit-author (lines 62-63 and 67-68) uses a double-assignment pattern that overwrites the author name variable with an email string. When commit-author: github-actions or commit-author: me is passed, the subsequent sed parsing produces the email address as both the author name and email, resulting in incorrect Git commit metadata (e.g. author name shows as github-actions[bot]@users.noreply.github.com instead of github-actions[bot]). This is a functional bug, not a security vulnerability, but it ships as-is from the upstream. Fix suggestions are attached as inline comments.

No other security vulnerabilities found.


Summary

The onboarding is complete and well-structured - all mandatory StepSecurity checks pass. Two items should be addressed before merging: remove the template CHANGE ME comment in _main.mjs, and fix the double-assignment logic bug in main.sh that causes incorrect Git author metadata when the github-actions or me shorthand values are used.

@anurag-stepsecurity
anurag-stepsecurity merged commit c199e9e into main Jul 17, 2026
6 checks passed
@anurag-stepsecurity
anurag-stepsecurity deleted the release branch July 17, 2026 11:34
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