mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 01:50:13 +01:00
fix(ci): load the lockfile policy from the current base-branch tip (#19390)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Only trusted environments may change `pnpm-lock.yaml`. #19356 moved that rule into `.github/scripts/lockfile-trust.mjs`. > - The `pr-trusted.yml` policy job loads that script from trusted code, so a pull request cannot change the check that judges it. > - The job reads the trusted commit from `pull_request.base.sha`. GitHub does not move that value forward on every push. > - #17364 showed the result. Its recorded base was still the `master` commit from before #19356, so the job used the earlier rule. > - The earlier rule exempts any branch named `chore/refresh-lockfile`, including one from a fork. > - This pull request loads the policy from the first parent of GitHub's merge commit, which is the current base-branch tip. > - The benefit is that every pull request gets the current lockfile policy, including pull requests opened before #19356. ## Linked Issues or Issue Description Refs #19356, #17364. **What happened?** On #17364 at `482d76686e`, the "Block untrusted lockfile edits" step printed nothing and passed. `pull_request.base.sha` was `da3084a05d`, the `master` commit from before #19356. That commit has no `lockfile-trust.mjs`, so the step used the earlier rule. The merge commit that CI tested already contained #19356. **Expected behavior** The policy job runs the current `lockfile-trust.mjs` from `master` for every pull request. **Steps to reproduce** 1. Open a pull request before #19356 merged, or keep a pull request whose recorded base predates it. 2. Push a commit. GitHub keeps the old `pull_request.base.sha`. 3. The policy job uses the earlier rule. A fork branch named `chore/refresh-lockfile` can still change `pnpm-lock.yaml`. **Also found** In a local test, the script exited 0 without any check when it ran from a symlinked temporary directory. Its entry-point check compared a symlinked path with a real path. GitHub-hosted Linux runners do not use a symlinked `RUNNER_TEMP`, but the check must fail closed. ## What Changed - `pr-trusted.yml` "Block untrusted lockfile edits": - Load `lockfile-trust.mjs` from `$GITHUB_SHA^1`, the first parent of GitHub's merge commit. Do this only when `$GITHUB_SHA^2` is the PR head. Otherwise, keep `pull_request.base.sha`. - Give the script the same trusted base for its diff. - Fail the step if the script writes no `writer=` output. - `lockfile-trust.mjs`: compare real paths in the entry-point check. - `lockfile-trust.test.mjs`: run the real step script against a git history with a stale recorded base and a merge commit. Check that it blocks a contributor lockfile change on a `chore/refresh-lockfile` branch, allows a docs-only change, and falls back to the recorded base when no merge commit is present. - `doc/RELEASE-AUTOMATION-SETUP.md`: state where the job loads the policy from. ## Verification - `node --test '.github/scripts/tests/*.test.mjs'`: 709 passed. - The new stale-base test fails when the workflow change is removed. - The policy-step tests run with `RUNNER_TEMP` behind a symlink. They fail when the realpath fix is removed. - A test loads a policy that exits 0 without writing `writer=`. It fails when the fail-closed check is removed. - This pull request's own CI uses `pr-trusted.yml` from `master`. The new step takes effect after merge. ## Risks - If the checkout is not GitHub's merge commit, the step falls back to `pull_request.base.sha`, which is the current behavior. The `pull_request` event always checks out the merge commit by default. - The fail-closed check can fail a pull request if the script stops writing `writer=`. The script writes it on every path that returns. - Low risk otherwise. The change touches one workflow step, one script line, tests, and docs. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use and code execution, in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
a12fcb8f26
commit
18ff71bca3
4 files changed
+123
-13
No files matched your search
@@ -133,8 +133,8 @@ jobs:
|
||||
|
||||
# Only trusted environments write lock content: the lockfile refresh
|
||||
# workflow, and Dependabot when the lockfile bytes come from a verified
|
||||
# Dependabot commit. Run the policy from the base commit so a pull request
|
||||
# cannot rewrite the check that judges it.
|
||||
# Dependabot commit. Run the policy from the current base-branch tip so a
|
||||
# pull request cannot rewrite the check that judges it.
|
||||
- name: Block untrusted lockfile edits
|
||||
id: lockfile_writer
|
||||
env:
|
||||
@@ -146,9 +146,22 @@ jobs:
|
||||
HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }}
|
||||
HEAD_REF: ${{ github.head_ref }}
|
||||
run: |
|
||||
# GitHub does not move pull_request.base.sha forward on every push, so
|
||||
# it can name an old base-branch commit. The checkout is GitHub's merge
|
||||
# commit: its first parent is the current base-branch tip, and its
|
||||
# second parent is the PR head. Load the policy from that tip.
|
||||
trusted_base="$BASE_SHA"
|
||||
if [ "$(git rev-parse --verify --quiet "$GITHUB_SHA^2")" = "$HEAD_SHA" ]; then
|
||||
trusted_base="$(git rev-parse "$GITHUB_SHA^1")"
|
||||
fi
|
||||
policy="$RUNNER_TEMP/lockfile-trust.mjs"
|
||||
if git show "$BASE_SHA:.github/scripts/lockfile-trust.mjs" > "$policy" 2>/dev/null; then
|
||||
node "$policy" pr
|
||||
if git show "$trusted_base:.github/scripts/lockfile-trust.mjs" > "$policy" 2>/dev/null; then
|
||||
BASE_SHA="$trusted_base" node "$policy" pr
|
||||
# Fail closed if the policy did not run.
|
||||
if ! grep -q '^writer=' "$GITHUB_OUTPUT"; then
|
||||
echo "::error title=Lockfile policy::lockfile-trust.mjs did not report a lockfile writer."
|
||||
exit 1
|
||||
fi
|
||||
exit 0
|
||||
fi
|
||||
# The base predates lockfile-trust.mjs: keep the earlier rule.
|
||||
@@ -156,7 +169,7 @@ jobs:
|
||||
if [ "$HEAD_REF" = 'chore/refresh-lockfile' ] || [ "$PR_AUTHOR" = 'dependabot[bot]' ]; then
|
||||
exit 0
|
||||
fi
|
||||
changed="$(git diff --name-only "$BASE_SHA...$HEAD_SHA")"
|
||||
changed="$(git diff --name-only "$trusted_base...$HEAD_SHA")"
|
||||
if printf '%s\n' "$changed" | grep -qx 'pnpm-lock.yaml'; then
|
||||
echo "Do not commit pnpm-lock.yaml in pull requests. CI owns lockfile updates."
|
||||
exit 1
|
||||
|
||||
Reference in new issue
Block a user