diff --git a/.bumpy/cwd-flag.md b/.bumpy/cwd-flag.md new file mode 100644 index 0000000..d9ed041 --- /dev/null +++ b/.bumpy/cwd-flag.md @@ -0,0 +1,5 @@ +--- +'@varlock/bumpy': minor +--- + +Added a global `--cwd ` flag that runs bumpy as if it were started in ``. This makes the `pull_request_target` PR-check workflow safe against a previously-undocumented attack: a fork PR could commit a `bunfig.toml`/`.npmrc` that redirected where `bunx @varlock/bumpy` itself was fetched from (swapping in a malicious package at the pinned version). The recommended workflow now fetches and runs bumpy from a trusted base checkout and points it at the untrusted PR tree with `--cwd ./pr`, so package-manager config in the PR can no longer influence how bumpy is obtained. diff --git a/.github/workflows/bumpy-check.yaml b/.github/workflows/bumpy-check.yaml index c3b0a15..57cec27 100644 --- a/.github/workflows/bumpy-check.yaml +++ b/.github/workflows/bumpy-check.yaml @@ -20,18 +20,28 @@ permissions: jobs: # Fork PRs (untrusted): run the PUBLISHED bumpy and never execute the PR's code. # pull_request_target carries a write token + secrets, so building/running fork - # code here would be a privilege-escalation hole. `ci check` reads json/yaml only. + # code — OR letting the fork's bunfig.toml/.npmrc redirect where bumpy itself is + # fetched from — would be a privilege-escalation hole. We fetch+run bumpy from a + # trusted base checkout and point it at the PR tree with --cwd. `ci check` only + # reads json/yaml. See docs/github-actions.md for the full rationale. check-published: if: github.event.pull_request.head.repo.full_name != github.repository runs-on: ubuntu-latest steps: - # Check out the PR head so bumpy can read the PR's bump files, config, and package.json. - # We never execute this code! + # 1. Trusted base checkout — bunx runs from here, so the package-manager + # config in effect is ours, not the PR's. + - uses: actions/checkout@v6 + with: + ref: main + persist-credentials: false + # 2. Untrusted PR head into ./pr — only READ from here, never resolve/run. - uses: actions/checkout@v6 with: ref: ${{ github.event.pull_request.head.sha }} + path: pr + persist-credentials: false - uses: oven-sh/setup-bun@v2 - - run: bunx @varlock/bumpy@latest ci check + - run: bunx @varlock/bumpy@latest ci check --cwd ./pr env: GH_TOKEN: ${{ github.token }} diff --git a/bun.lock b/bun.lock index 14ab283..8f7dabf 100644 --- a/bun.lock +++ b/bun.lock @@ -14,7 +14,7 @@ }, "packages/bumpy": { "name": "@varlock/bumpy", - "version": "1.0.0", + "version": "1.16.1", "bin": { "bumpy": "./dist/cli.mjs", }, @@ -29,6 +29,7 @@ "picomatch": "^4.0.4", "semver": "^7.7.2", "tsdown": "catalog:", + "typescript": "catalog:", }, }, "packages/website": { @@ -185,7 +186,7 @@ "@tybys/wasm-util": ["@tybys/wasm-util@0.10.1", "", { "dependencies": { "tslib": "^2.4.0" } }, "sha512-9tTaPJLSiejZKx+Bmog4uSubteqTvFrVrURwkmHixBo0G4seD0zUxp98E1DzUBJxLQ3NPwXrGKDiVjwx/DpPsg=="], - "@types/bun": ["@types/bun@1.3.12", "", { "dependencies": { "bun-types": "1.3.12" } }, "sha512-DBv81elK+/VSwXHDlnH3Qduw+KxkTIWi7TXkAeh24zpi5l0B2kUg9Ga3tb4nJaPcOFswflgi/yAvMVBPrxMB+A=="], + "@types/bun": ["@types/bun@1.3.14", "", { "dependencies": { "bun-types": "1.3.14" } }, "sha512-h1hFqFVcvAvD9j9K7ZW7vd82aSA+rTdznZa+5bwvCwqSB1jmmfLcbIWhOLx1/+boy/xmjgCs/OMUL8hRJSmnPw=="], "@types/estree": ["@types/estree@1.0.8", "", {}, "sha512-dWHzHa2WqEXI/O1E9OjrocMTKJl2mSrEolh1Iomrv6U+JuNwaHXsXx9bLu5gG7BUWFIN0skIQJQ/L1rIex4X6w=="], @@ -211,7 +212,7 @@ "birpc": ["birpc@4.0.0", "", {}, "sha512-LShSxJP0KTmd101b6DRyGBj57LZxSDYWKitQNW/mi8GRMvZb078Uf9+pveax1DrVL89vm7mWe+TovdI/UDOuPw=="], - "bun-types": ["bun-types@1.3.12", "", { "dependencies": { "@types/node": "*" } }, "sha512-HqOLj5PoFajAQciOMRiIZGNoKxDJSr6qigAttOX40vJuSp6DN/CxWp9s3C1Xwm4oH7ybueITwiaOcWXoYVoRkA=="], + "bun-types": ["bun-types@1.3.14", "", { "dependencies": { "@types/node": "*" } }, "sha512-4N0ig0fEomHt5R0KCFWjovxow98rIoRwKolrYdCcknNwMekCXRnWEUvgu5soYV8QXtVsrUD8B95MBOZGPvr6KQ=="], "cac": ["cac@7.0.0", "", {}, "sha512-tixWYgm5ZoOD+3g6UTea91eow5z6AAHaho3g0V9CNSNb45gM8SmflpAc+GRd1InC4AqN/07Unrgp56Y94N9hJQ=="], @@ -299,7 +300,7 @@ "tslib": ["tslib@2.8.1", "", {}, "sha512-oJFu94HQb+KVduSUQL7wnpmqnfmLsOA/nAh6b6EH0wCEoK0/mPeXU6c3wKDV83MkOuHPRHtSXKKU99IBazS/2w=="], - "typescript": ["typescript@5.9.3", "", { "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" } }, "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw=="], + "typescript": ["typescript@6.0.3", "", { "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" } }, "sha512-y2TvuxSZPDyQakkFRPZHKFm+KKVqIisdg9/CZwm9ftvKXLP8NRWj38/ODjNbr43SsoXqNuAisEf1GdCxqWcdBw=="], "unconfig-core": ["unconfig-core@7.5.0", "", { "dependencies": { "@quansync/fs": "^1.0.0", "quansync": "^1.0.0" } }, "sha512-Su3FauozOGP44ZmKdHy2oE6LPjk51M/TRRjHv2HNCWiDvfvCoxC2lno6jevMA91MYAdCdwP05QnWdWpSbncX/w=="], diff --git a/docs/github-actions.md b/docs/github-actions.md index 57765a1..708d173 100644 --- a/docs/github-actions.md +++ b/docs/github-actions.md @@ -18,7 +18,7 @@ These commands facilitate the following: ## PR check workflow -Posts/updates the release-plan comment on every PR, including PRs from forks. Adapt as needed — but **do not add an install step or run any PR-defined scripts** (see the security note after the example). +Posts/updates the release-plan comment on every PR, including PRs from forks. **Copy it as-is and you're covered** — it's structured so nothing in a fork PR can influence how bumpy is fetched or run. (If you change `main` to your own base branch, that's the only edit most repos need. See the security notes below before restructuring it.) ```yaml # .github/workflows/bumpy-check.yaml @@ -34,58 +34,87 @@ jobs: check: runs-on: ubuntu-latest steps: - # Check out the PR head so bumpy can read the PR's bump files, config, - # and package.json. We never execute this code. + # 1. TRUSTED checkout of the base branch — bumpy is fetched and run from + # here, so the package-manager config in effect (bunfig.toml, .npmrc) + # is YOURS, not the PR's. Hardcoded "main" (the PR controls its own + # base ref); change it to your base branch. + - uses: actions/checkout@v6 + with: + ref: main + persist-credentials: false + + # 2. UNTRUSTED PR head into ./pr — only READ from here, never run it. - uses: actions/checkout@v6 with: ref: ${{ github.event.pull_request.head.sha }} + path: pr + persist-credentials: false + - uses: oven-sh/setup-bun@v2 - # ⚠️ DO NOT INSTALL DEPS OR EXECUTE CODE ⚠️ + # ⚠️ DO NOT bun install / npm install / run any script from ./pr ⚠️ - # Resolve bumpy's version from the BASE branch's package.json (trusted). - # Reading it from the PR's package.json would let a fork PR swap in a - # malicious version of bumpy. + # Read bumpy's version from the TRUSTED base checkout, never from ./pr. - name: Resolve bumpy version from base run: | - # Hardcoded to "main" rather than ${{ github.event.pull_request.base.ref }} - # because the PR controls its base — pointing at any other branch you have - # would read that branch's package.json. Change "main" to your base branch. - git fetch origin main --depth=1 - VERSION=$(git show "origin/main:package.json" \ - | jq -r '.devDependencies["@varlock/bumpy"] // .dependencies["@varlock/bumpy"]' \ - | sed 's/[\^~]//') + VERSION=$(jq -r '.devDependencies["@varlock/bumpy"] // .dependencies["@varlock/bumpy"]' package.json | sed 's/[\^~]//') echo "BUMPY_VERSION=$VERSION" >> "$GITHUB_ENV" - # Quote the version arg so a malformed value can't shell-inject. - - run: bunx "@varlock/bumpy@$BUMPY_VERSION" ci check + # bunx runs from the trusted root; `--cwd ./pr` only points bumpy at the + # PR tree to read its bump files. Quote the version arg against injection. + - run: bunx "@varlock/bumpy@$BUMPY_VERSION" ci check --cwd ./pr env: GH_TOKEN: ${{ github.token }} ``` -### ⚠️ Security: no installs, no PR scripts +### ⚠️ Security essentials + +`pull_request_target` carries a **write token and secrets even on fork PRs** — that's what lets it comment on forks, and why a PR author must never be able to influence what runs. The workflow above handles this; two rules to preserve if you adapt it: -`pull_request_target` runs with write permissions and access to secrets — even on fork PRs. That's what lets us post comments on PRs from forks, but it means the workflow must never execute code that a PR author controls. In practice: +- **Never execute PR code** — no `bun install` / `npm install` (postinstall scripts run), no `bun run