diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f60b01f75..633581fd6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -130,14 +130,14 @@ jobs: echo "Coverage itself is gated by Codecov on changed lines (codecov/patch), computed from the complete lcov generated by this job." echo "Reproduce locally with: 'npm run test:coverage'." - name: Verify coverage report exists - if: ${{ success() && (github.event_name == 'push' || needs.changes.outputs.backend == 'true') && github.event.pull_request.head.repo.fork != true }} + if: ${{ success() && (github.event_name == 'push' || needs.changes.outputs.backend == 'true') }} run: | if [ ! -s coverage/lcov.info ]; then echo "::error title=Coverage::coverage/lcov.info is missing or empty" exit 1 fi - # Direct upload for trusted contexts only (push + same-repo PRs), where secrets.CODECOV_TOKEN is - # available. Fork PRs skip this — they cannot read the token — and take the artifact path below. + # Direct upload for trusted contexts (push + same-repo PRs), where secrets.CODECOV_TOKEN is + # available. - name: Upload coverage to Codecov if: ${{ success() && (github.event_name == 'push' || needs.changes.outputs.backend == 'true') && github.event.pull_request.head.repo.fork != true }} uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 @@ -151,6 +151,24 @@ jobs: # Coverage upload is part of the hard gate: upload or service errors # should fail CI instead of allowing a PR to merge without patch data. fail_ci_if_error: true + # Fork PRs run without secrets, so they cannot use the token path above. codecov-action v4+ supports + # tokenless upload for public repos, but its commit auto-detection cannot be trusted here: for + # pull_request events GITHUB_SHA is the ephemeral auto-merge commit, and codecov-cli's fallback to + # recover the real head sha assumes HEAD is that 2-parent merge commit -- our checkout step above + # deliberately checks out github.event.pull_request.head.sha directly (so tests run the contributor's + # actual commit, not a synthetic merge), so HEAD has one parent and that recovery can't fire. Pass + # the same explicit overrides as the trusted upload above so the report attaches to the real PR head, + # not a merge sha GitHub's PR checks list has no reason to display. + - name: Upload coverage to Codecov (fork PR tokenless) + if: ${{ success() && needs.changes.outputs.backend == 'true' && github.event.pull_request.head.repo.fork == true }} + uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 + with: + files: ./coverage/lcov.info + disable_search: true + override_branch: ${{ github.event.pull_request.head.ref }} + override_commit: ${{ github.event.pull_request.head.sha }} + override_pr: ${{ github.event.pull_request.number }} + fail_ci_if_error: true # Test results are useful Codecov annotations, not the coverage gate. Keep # their upload non-blocking so a JUnit ingestion hiccup does not fail CI # after the tests and hard coverage upload have already passed. @@ -166,33 +184,17 @@ jobs: override_commit: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }} override_pr: ${{ github.event_name == 'pull_request' && github.event.pull_request.number || '' }} fail_ci_if_error: false - # Fork PRs run without secrets, so they cannot upload to Codecov here. Stash the coverage report and the - # PR metadata as an artifact; the privileged `Codecov fork upload` workflow (workflow_run, base-repo - # context, has the token) uploads it — keeping codecov/patch enforced on forks. Fork-controlled values - # (branch name) go through env, never shell interpolation, and are re-validated before use downstream. - - name: Stash fork coverage for out-of-band upload - if: ${{ success() && needs.changes.outputs.backend == 'true' && github.event.pull_request.head.repo.fork == true }} - env: - PR_NUMBER: ${{ github.event.pull_request.number }} - HEAD_SHA: ${{ github.event.pull_request.head.sha }} - HEAD_BRANCH: ${{ github.event.pull_request.head.ref }} - run: | - mkdir -p fork-coverage - cp coverage/lcov.info fork-coverage/lcov.info - cp reports/junit/vitest.xml fork-coverage/vitest.xml 2>/dev/null || true - { - printf 'PR_NUMBER=%s\n' "$PR_NUMBER" - printf 'HEAD_SHA=%s\n' "$HEAD_SHA" - printf 'HEAD_BRANCH=%s\n' "$HEAD_BRANCH" - } > fork-coverage/codecov-meta.env - - name: Upload fork coverage artifact - if: ${{ success() && needs.changes.outputs.backend == 'true' && github.event.pull_request.head.repo.fork == true }} - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + - name: Upload Vitest results to Codecov (fork PR tokenless) + if: ${{ !cancelled() && needs.changes.outputs.backend == 'true' && github.event.pull_request.head.repo.fork == true }} + uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 with: - name: fork-coverage - path: fork-coverage/ - retention-days: 1 - if-no-files-found: error + files: ./reports/junit/vitest.xml + report_type: test_results + disable_search: true + override_branch: ${{ github.event.pull_request.head.ref }} + override_commit: ${{ github.event.pull_request.head.sha }} + override_pr: ${{ github.event.pull_request.number }} + fail_ci_if_error: false - name: Worker runtime tests if: ${{ github.event_name == 'push' || needs.changes.outputs.backend == 'true' }} run: npm run test:workers diff --git a/.github/workflows/codecov-fork-upload.yml b/.github/workflows/codecov-fork-upload.yml deleted file mode 100644 index bd0da3d4a..000000000 --- a/.github/workflows/codecov-fork-upload.yml +++ /dev/null @@ -1,84 +0,0 @@ -name: Codecov fork upload - -# Fork PR CI runs without access to secrets, so ci.yml cannot upload their coverage to Codecov directly — -# it stashes the coverage report + PR metadata as the `fork-coverage` artifact. This workflow runs on -# `workflow_run` in the BASE-repo context (so it CAN read secrets.CODECOV_TOKEN) and performs the upload, -# keeping the codecov/patch gate enforced on fork PRs. -# -# Security: this is a privileged workflow processing a fork-produced artifact. It never checks out or runs -# fork code — it only downloads the artifact and passes validated metadata to codecov-action. The job runs -# ONLY for successful fork-PR CI runs, and every fork-controlled value is re-validated against a strict -# charset before use. -on: - workflow_run: - workflows: ["CI"] - types: [completed] - -permissions: - contents: read - actions: read - -jobs: - upload: - if: >- - ${{ github.event.workflow_run.conclusion == 'success' - && github.event.workflow_run.event == 'pull_request' - && github.event.workflow_run.head_repository.full_name != github.repository }} - runs-on: ubuntu-latest - timeout-minutes: 10 - steps: - - name: Download fork coverage artifact - id: download - continue-on-error: true - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 - with: - name: fork-coverage - path: fork-coverage - run-id: ${{ github.event.workflow_run.id }} - github-token: ${{ github.token }} - - name: Read and validate coverage metadata - id: meta - run: | - f=fork-coverage/codecov-meta.env - if [ ! -f "$f" ]; then - echo "no fork-coverage artifact on this run — nothing to upload" - echo "skip=true" >> "$GITHUB_OUTPUT" - exit 0 - fi - pr="$(sed -n 's/^PR_NUMBER=//p' "$f" | head -1)" - sha="$(sed -n 's/^HEAD_SHA=//p' "$f" | head -1)" - branch="$(sed -n 's/^HEAD_BRANCH=//p' "$f" | head -1)" - # Reject anything that is not a plain PR number / hex sha / safe ref name — a fork controls these. - case "$pr" in '' | *[!0-9]*) echo "invalid PR number"; echo "skip=true" >> "$GITHUB_OUTPUT"; exit 0 ;; esac - case "$sha" in '' | *[!0-9a-fA-F]*) echo "invalid head sha"; echo "skip=true" >> "$GITHUB_OUTPUT"; exit 0 ;; esac - case "$branch" in '' | *[!A-Za-z0-9._/-]*) echo "invalid head branch"; echo "skip=true" >> "$GITHUB_OUTPUT"; exit 0 ;; esac - { - echo "skip=false" - echo "pr=$pr" - echo "sha=$sha" - echo "branch=$branch" - } >> "$GITHUB_OUTPUT" - - name: Upload coverage to Codecov - if: ${{ steps.meta.outputs.skip == 'false' }} - uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 - with: - token: ${{ secrets.CODECOV_TOKEN }} - files: fork-coverage/lcov.info - disable_search: true - override_branch: ${{ steps.meta.outputs.branch }} - override_commit: ${{ steps.meta.outputs.sha }} - override_pr: ${{ steps.meta.outputs.pr }} - # Same hard-gate semantics as the inline upload: a failed upload must not silently drop patch data. - fail_ci_if_error: true - - name: Upload Vitest results to Codecov - if: ${{ steps.meta.outputs.skip == 'false' && hashFiles('fork-coverage/vitest.xml') != '' }} - uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 - with: - token: ${{ secrets.CODECOV_TOKEN }} - files: fork-coverage/vitest.xml - report_type: test_results - disable_search: true - override_branch: ${{ steps.meta.outputs.branch }} - override_commit: ${{ steps.meta.outputs.sha }} - override_pr: ${{ steps.meta.outputs.pr }} - fail_ci_if_error: false diff --git a/test/unit/codecov-policy.test.ts b/test/unit/codecov-policy.test.ts index 9d89fb8b7..c0ef6edbd 100644 --- a/test/unit/codecov-policy.test.ts +++ b/test/unit/codecov-policy.test.ts @@ -52,7 +52,11 @@ describe("Codecov policy", () => { const coverageUpload = steps[coverageUploadIndex]!; const testResultsUpload = steps[testResultsUploadIndex]!; - expect(verifyStep.if).toBe(coverageUpload.if); + // Verify must run whenever coverage was generated at all (push or backend==true) -- it deliberately + // does NOT exclude forks, since both the trusted and the tokenless fork upload path below it need + // the report to exist first. + expect(String(verifyStep.if)).toBe("${{ success() && (github.event_name == 'push' || needs.changes.outputs.backend == 'true') }}"); + expect(String(coverageUpload.if)).toContain(String(verifyStep.if).replace(/^\$\{\{\s*|\s*\}\}$/g, "")); expect(String(verifyStep.run)).toContain("coverage/lcov.info is missing or empty"); expect(String(verifyStep.run)).toContain("exit 1"); @@ -66,4 +70,56 @@ describe("Codecov policy", () => { expect(testResultsUploadWith.disable_search).toBe(true); expect(testResultsUploadWith.fail_ci_if_error).toBe(false); }); + + it("uploads fork PR coverage tokenlessly instead of silently skipping it", () => { + // Fork PRs cannot read secrets.CODECOV_TOKEN. Previously the token-gated upload steps simply + // excluded forks with no replacement, so codecov/patch had no report to compare against and fell + // back to Codecov's if_not_found: success default -- a green "0.00%, not affected" check that never + // actually enforced the patch bar on fork contributions. codecov-action's tokenless upload path + // (public repos only) closes that gap with a single, synchronous, same-job upload: no separate + // workflow, no artifact staging, no fork-authored attribution data to trust or validate. + const workflow = readYaml(".github/workflows/ci.yml"); + const validateCode = nestedRecord(workflow, ["jobs", "validate-code"]); + const steps = recordArray(validateCode.steps, "jobs.validate-code.steps"); + + const verifyStep = steps.find((step) => step.name === "Verify coverage report exists"); + expect(verifyStep).toBeDefined(); + // The existence check must apply to forks too now -- it used to explicitly exclude them. + expect(String(verifyStep!.if)).not.toContain("fork"); + + const forkCoverageUpload = steps.find((step) => step.name === "Upload coverage to Codecov (fork PR tokenless)"); + expect(forkCoverageUpload).toBeDefined(); + expect(String(forkCoverageUpload!.if)).toContain("github.event.pull_request.head.repo.fork == true"); + + const forkCoverageWith = record(forkCoverageUpload!.with, "fork coverage upload with"); + expect(forkCoverageWith.token).toBeUndefined(); + expect(forkCoverageWith.files).toBe("./coverage/lcov.info"); + expect(forkCoverageWith.disable_search).toBe(true); + expect(forkCoverageWith.fail_ci_if_error).toBe(true); + // GITHUB_SHA is the ephemeral auto-merge commit on pull_request events, and codecov-cli's fallback to + // recover the real head sha assumes a 2-parent merge commit at HEAD -- which our checkout step (it + // fetches github.event.pull_request.head.sha directly) never produces. Without an explicit override, + // the report would attach to a sha GitHub's PR checks list has no reason to ever display. + expect(forkCoverageWith.override_branch).toBe("${{ github.event.pull_request.head.ref }}"); + expect(forkCoverageWith.override_commit).toBe("${{ github.event.pull_request.head.sha }}"); + expect(forkCoverageWith.override_pr).toBe("${{ github.event.pull_request.number }}"); + + const forkTestResultsUpload = steps.find( + (step) => step.name === "Upload Vitest results to Codecov (fork PR tokenless)", + ); + expect(forkTestResultsUpload).toBeDefined(); + const forkTestResultsWith = record(forkTestResultsUpload!.with, "fork test results upload with"); + expect(forkTestResultsWith.token).toBeUndefined(); + expect(forkTestResultsWith.report_type).toBe("test_results"); + expect(forkTestResultsWith.fail_ci_if_error).toBe(false); + expect(forkTestResultsWith.override_commit).toBe("${{ github.event.pull_request.head.sha }}"); + + // The trusted (token) path must still explicitly exclude forks -- it must never see the token env + // used, and the two paths must be mutually exclusive so a fork PR never double-uploads. + const trustedCoverageUpload = steps.find((step) => step.name === "Upload coverage to Codecov"); + expect(trustedCoverageUpload).toBeDefined(); + expect(String(trustedCoverageUpload!.if)).toContain("github.event.pull_request.head.repo.fork != true"); + const trustedWith = record(trustedCoverageUpload!.with, "trusted coverage upload with"); + expect(trustedWith.token).toBe("${{ secrets.CODECOV_TOKEN }}"); + }); });