diff --git a/.github/scripts/check-pr-security.mjs b/.github/scripts/check-pr-security.mjs index 8d3ac9ab..dd2ea53f 100644 --- a/.github/scripts/check-pr-security.mjs +++ b/.github/scripts/check-pr-security.mjs @@ -225,10 +225,14 @@ export async function syncDraftAdvisory(fetchImpl, token, repo, prNumber, prTitl throw new Error(`Existing advisory for PR #${prNumber} is missing both ghsa_id and id.`); } + // PATCH rejects `vulnerabilities: []` with 422 ("Advisory must have at least one vulnerability"). + // The field is only valid on POST when creating the draft; updates must omit it. + const { vulnerabilities, ...patchPayload } = payload; + return fetchImpl(`/repos/${repo}/security-advisories/${advisoryId}`, token, { method: 'PATCH', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(payload), + body: JSON.stringify(patchPayload), }); } @@ -266,10 +270,16 @@ export async function postSecurityCheckRun(fetchImpl, token, repo, headSha, hasF body: JSON.stringify(hasFlags ? { name: 'security-review', head_sha: headSha, - status: 'in_progress', + // `completed/neutral` instead of `in_progress` so the check doesn't put + // the PR in `mergeStateStatus: BLOCKED`. The draft advisory is the + // durable signal for maintainers; there is no completion path that + // could ever flip an `in_progress` check-run back to completed on the + // same head SHA, so it would hang forever. + status: 'completed', + conclusion: 'neutral', output: { - title: 'Security Review Pending', - summary: 'This PR has been flagged for manual security review by a maintainer. No action needed from you.', + title: 'Security Review Recommended', + summary: 'Draft advisory filed for maintainer review. Not a merge block — review the advisory at your leisure.', }, } : { name: 'security-review', diff --git a/.github/scripts/tests/check-pr-security.test.mjs b/.github/scripts/tests/check-pr-security.test.mjs index d1283333..26667750 100644 --- a/.github/scripts/tests/check-pr-security.test.mjs +++ b/.github/scripts/tests/check-pr-security.test.mjs @@ -161,7 +161,10 @@ test('syncDraftAdvisory: patches an existing advisory with the latest flags', as assert.equal(calls.length, 2); assert.equal(calls[1].path, '/repos/paperclipai/paperclip/security-advisories/GHSA-test-1234'); assert.equal(calls[1].options.method, 'PATCH'); - assert.deepEqual(JSON.parse(calls[1].options.body), buildAdvisoryPayload(6469, 'My PR', flags)); + const patchBody = JSON.parse(calls[1].options.body); + const { vulnerabilities, ...expectedPatch } = buildAdvisoryPayload(6469, 'My PR', flags); + assert.deepEqual(patchBody, expectedPatch); + assert.ok(!('vulnerabilities' in patchBody), 'PATCH must omit vulnerabilities (GitHub rejects empty array with 422)'); }); test('syncDraftAdvisory: creates a new advisory when none exists', async () => { @@ -196,10 +199,11 @@ test('postSecurityCheckRun: uses the injected fetch implementation', async () => assert.deepEqual(JSON.parse(calls[0].options.body), { name: 'security-review', head_sha: 'deadbeef', - status: 'in_progress', + status: 'completed', + conclusion: 'neutral', output: { - title: 'Security Review Pending', - summary: 'This PR has been flagged for manual security review by a maintainer. No action needed from you.', + title: 'Security Review Recommended', + summary: 'Draft advisory filed for maintainer review. Not a merge block — review the advisory at your leisure.', }, }); }); diff --git a/.github/workflows/commitperclip-review.yml b/.github/workflows/commitperclip-review.yml index 2e385d1f..371e9ee9 100644 --- a/.github/workflows/commitperclip-review.yml +++ b/.github/workflows/commitperclip-review.yml @@ -45,6 +45,7 @@ jobs: - name: Run quality gates id: quality + if: github.event.pull_request.user.login != 'dependabot[bot]' run: node .github/scripts/run-quality-gates.mjs continue-on-error: true env: @@ -63,7 +64,9 @@ jobs: PR_AUTHOR: ${{ github.event.pull_request.user.login }} - name: Fail if quality gates failed - if: steps.quality.outcome == 'failure' + if: >- + github.event.pull_request.user.login != 'dependabot[bot]' && + steps.quality.outcome == 'failure' run: | echo "One or more quality gates failed. See commitperclip comment on the PR for details." exit 1 diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 4878c308..b6051aac 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -21,7 +21,9 @@ jobs: fetch-depth: 0 - name: Block manual lockfile edits - if: github.head_ref != 'chore/refresh-lockfile' + if: >- + github.head_ref != 'chore/refresh-lockfile' && + github.event.pull_request.user.login != 'dependabot[bot]' run: | # Diff the PR branch against its merge base so recent base-branch commits # do not masquerade as changes made by the PR itself.