diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 4470b5763fcf84f54ea1b0ef7c2f7bf9786eaaca..b8b7939813f9cc72da88e75653b6f2933403a239 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,10 +1,28 @@ -Closes #ISSUE +## Context -Before you mark this PR as ready for review, make sure that you have: -- [ ] Added a solid test coverage and/or screenshots from doing manual testing -- [ ] Done a self-review taking into account security and performance aspects -- [ ] Aligned any UI changes with the [UI checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) + + +## How to Review + + + +## Self-Review Checklist + + +- [ ] I've reviewed my own diff for quality, security, and reliability +- [ ] Unsafe blocks (if any) have justifying comments +- [ ] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) +- [ ] Tests cover the new/changed behavior +- [ ] Performance impact has been considered and is acceptable Release Notes: -- N/A *or* Added/Fixed/Improved ... +- N/A or Added/Fixed/Improved ... diff --git a/.github/workflows/assign-reviewers.yml b/.github/workflows/assign-reviewers.yml index a77f1812d06330b4635fe173583f0f1ce93e4e17..1242caa61ed8ef5066394936bfcd7f1b5800e416 100644 --- a/.github/workflows/assign-reviewers.yml +++ b/.github/workflows/assign-reviewers.yml @@ -10,25 +10,43 @@ # AUTH NOTE: Uses a GitHub App (COORDINATOR_APP_ID + COORDINATOR_APP_PRIVATE_KEY) # for all API operations: cloning the private coordinator repo, requesting team # reviewers, and setting PR assignees. GITHUB_TOKEN is not used. +# +# SECURITY INVARIANTS (pull_request_target): +# This workflow runs with access to secrets for ALL PRs including forks. +# It is safe ONLY because: +# 1. The checkout is the coordinator repo at ref: main — NEVER the PR head/branch +# 2. No ${{ }} interpolation of event fields in run: blocks — all routed via env: +# 3. The script never executes, sources, or reads files from the PR branch +# Violating any of these enables remote code execution with secret access. name: Assign Reviewers on: - pull_request: + # zizmor: ignore[dangerous-triggers] reviewed — no PR code checkout, only coordinator repo at ref: main + pull_request_target: types: [opened, ready_for_review] # GITHUB_TOKEN is not used — all operations use the GitHub App token. # Declare minimal permissions so the default token has no write access. permissions: {} -# Only run for PRs from within the org (not forks) — fork PRs don't have -# write access to request team reviewers. +# Prevent duplicate runs for the same PR (e.g., rapid push + ready_for_review). +concurrency: + group: assign-reviewers-${{ github.event.pull_request.number }} + cancel-in-progress: true + +# NOTE: For ready_for_review events, the webhook payload may still carry +# draft: true due to a GitHub race condition (payload serialized before DB +# update). We trust the event type instead — the script rechecks draft status +# via a live API call as defense-in-depth. +# +# No author_association filter — external and fork PRs also get reviewer +# assignments. Assigned reviewers are inherently scoped to org team members +# by the GitHub Teams API. jobs: assign-reviewers: if: >- - github.event.pull_request.head.repo.full_name == github.repository && - github.event.pull_request.draft == false && - contains(fromJSON('["MEMBER", "OWNER"]'), github.event.pull_request.author_association) + github.event.action == 'ready_for_review' || github.event.pull_request.draft == false runs-on: ubuntu-latest steps: - name: Generate app token @@ -39,6 +57,8 @@ jobs: private-key: ${{ secrets.COORDINATOR_APP_PRIVATE_KEY }} repositories: codeowner-coordinator,zed + # SECURITY: checks out the coordinator repo at ref: main, NOT the PR branch. + # persist-credentials: false prevents the token from leaking into .git/config. - name: Checkout coordinator repo uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 with: @@ -54,7 +74,10 @@ jobs: python-version: "3.11" - name: Install dependencies - run: pip install pyyaml==6.0.3 + run: | + pip install pyyaml==6.0.3 \ + --require-hashes --no-deps -q --only-binary ':all:' \ + -c /dev/stdin <<< "pyyaml==6.0.3 --hash=sha256:b8bb0864c5a28024fac8a632c443c87c5aa6f215c0b126c449ae1a150412f31d" - name: Assign reviewers env: @@ -69,7 +92,6 @@ jobs: --rules-file team-membership-rules.yml \ --repo "$TARGET_REPO" \ --org zed-industries \ - --min-association member \ 2>&1 | tee /tmp/assign-reviewers-output.txt - name: Upload output diff --git a/.github/workflows/hotfix-review-monitor.yml b/.github/workflows/hotfix-review-monitor.yml new file mode 100644 index 0000000000000000000000000000000000000000..e672a8506c1a103524f097dc36fc8ceee4ccba00 --- /dev/null +++ b/.github/workflows/hotfix-review-monitor.yml @@ -0,0 +1,115 @@ +# Hotfix Review Monitor +# +# Runs daily and checks for merged PRs with the 'hotfix' label that have not +# received a post-merge review approval within one business day. Posts a summary to +# Slack if any are found. This is a SOC2 compensating control for the +# emergency hotfix fast path. +# +# Security note: No untrusted input (PR titles, bodies, etc.) is interpolated +# into shell commands. All PR metadata is read via gh API + jq, not via +# github.event context expressions. +# +# Required secrets: +# SLACK_WEBHOOK_PR_REVIEW_BOT - Incoming webhook URL for the #pr-review-ops channel + +name: Hotfix Review Monitor + +on: + schedule: + - cron: "30 13 * * 1-5" # 1:30 PM UTC weekdays + workflow_dispatch: {} + +permissions: + contents: read + pull-requests: read + +jobs: + check-hotfix-reviews: + if: github.repository_owner == 'zed-industries' + runs-on: ubuntu-latest + timeout-minutes: 5 + env: + REPO: ${{ github.repository }} + steps: + - name: Find unreviewed hotfixes + id: check + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + # 80h lookback covers the Friday-to-Monday gap (72h) with buffer. + # Overlap on weekdays is harmless — reviewed PRs are filtered out below. + SINCE=$(date -u -v-80H +%Y-%m-%dT%H:%M:%SZ 2>/dev/null \ + || date -u -d '80 hours ago' +%Y-%m-%dT%H:%M:%SZ) + + # Get merged PRs with hotfix label from the lookback window + gh api --paginate \ + "repos/${REPO}/pulls?state=closed&sort=updated&direction=desc&per_page=50" \ + --jq "[ + .[] | + select(.merged_at != null) | + select(.merged_at > \"$SINCE\") | + select(.labels | map(.name) | index(\"hotfix\")) + ]" > /tmp/hotfix_prs.json + + # Check each hotfix PR for a post-merge approving review + jq -r '.[].number' /tmp/hotfix_prs.json | while read -r PR_NUMBER; do + APPROVALS=$(gh api \ + "repos/${REPO}/pulls/${PR_NUMBER}/reviews" \ + --jq "[.[] | select(.state == \"APPROVED\")] | length") + + if [ "$APPROVALS" -eq 0 ]; then + jq ".[] | select(.number == ${PR_NUMBER}) | {number, title, merged_at}" \ + /tmp/hotfix_prs.json + fi + done | jq -s '.' > /tmp/unreviewed.json + + COUNT=$(jq 'length' /tmp/unreviewed.json) + echo "count=$COUNT" >> "$GITHUB_OUTPUT" + + - name: Notify Slack + if: steps.check.outputs.count != '0' + env: + SLACK_WEBHOOK_PR_REVIEW_BOT: ${{ secrets.SLACK_WEBHOOK_PR_REVIEW_BOT }} + COUNT: ${{ steps.check.outputs.count }} + run: | + # Build Block Kit payload from JSON — no shell interpolation of PR titles. + # Why jq? PR titles are attacker-controllable input. By reading them + # through jq -r from the JSON file and passing the result to jq --arg, + # the content stays safely JSON-encoded in the final payload. Block Kit + # doesn't change this — the same jq pipeline feeds into the blocks + # structure instead of plain text. + PRS=$(jq -r '.[] | "• — \(.title) (merged \(.merged_at | split("T")[0]))"' /tmp/unreviewed.json) + + jq -n \ + --arg count "$COUNT" \ + --arg prs "$PRS" \ + '{ + text: ($count + " hotfix PR(s) still need post-merge review"), + blocks: [ + { + type: "section", + text: { + type: "mrkdwn", + text: (":rotating_light: *" + $count + " Hotfix PR(s) Need Post-Merge Review*") + } + }, + { + type: "section", + text: { type: "mrkdwn", text: $prs } + }, + { type: "divider" }, + { + type: "context", + elements: [{ + type: "mrkdwn", + text: "Hotfix PRs require review within one business day of merge." + }] + } + ] + }' | \ + curl -s -X POST "$SLACK_WEBHOOK_PR_REVIEW_BOT" \ + -H 'Content-Type: application/json' \ + -d @- +defaults: + run: + shell: bash -euxo pipefail {0} diff --git a/.github/workflows/pr-size-check.yml b/.github/workflows/pr-size-check.yml new file mode 100644 index 0000000000000000000000000000000000000000..215bb6cfd8076364db5361df90c28ac1e13f8178 --- /dev/null +++ b/.github/workflows/pr-size-check.yml @@ -0,0 +1,172 @@ +# PR Size Check +# +# Comments on PRs that exceed the 400 LOC soft limit with a friendly reminder +# to consider splitting. Does NOT block the PR — advisory only. +# Also adds size labels (size/S, size/M, size/L, size/XL) for tracking. +# +# Security note: Uses actions/github-script (JavaScript API) — no shell +# interpolation of untrusted input. PR body is accessed via the JS API, +# not via expression interpolation in run: blocks. + +name: PR Size Check + +on: + pull_request: + types: [opened, synchronize] + +permissions: + contents: read + +jobs: + check-size: + if: github.repository_owner == 'zed-industries' + permissions: + contents: read + pull-requests: write # PR comments + issues: write # label management (GitHub routes labels through Issues API) + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Calculate PR size and label + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 + with: + script: | + const { data: files } = await github.rest.pulls.listFiles({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.issue.number, + per_page: 300, + }); + + // Sum additions + deletions, excluding generated/lock files + const IGNORED_PATTERNS = [ + /\.lock$/, + /^Cargo\.lock$/, + /pnpm-lock\.yaml$/, + /\.generated\./, + /\/fixtures\//, + /\/snapshots\//, + ]; + + let totalChanges = 0; + for (const file of files) { + const ignored = IGNORED_PATTERNS.some(p => p.test(file.filename)); + if (!ignored) { + totalChanges += file.additions + file.deletions; + } + } + + // Assign size label + const SIZE_BRACKETS = [ + ['size/S', 0, 100, '0e8a16'], + ['size/M', 100, 400, 'fbca04'], + ['size/L', 400, 800, 'e99695'], + ['size/XL', 800, Infinity, 'b60205'], + ]; + + let sizeLabel = 'size/S'; + let labelColor = '0e8a16'; + for (const [label, min, max, color] of SIZE_BRACKETS) { + if (totalChanges >= min && totalChanges < max) { + sizeLabel = label; + labelColor = color; + break; + } + } + + // Remove existing size labels, then apply the current one + const existingLabels = (await github.rest.issues.listLabelsOnIssue({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + })).data.map(l => l.name); + + for (const label of existingLabels) { + if (label.startsWith('size/')) { + await github.rest.issues.removeLabel({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + name: label, + }); + } + } + + // Create the label if it doesn't exist (ignore 422 = already exists) + try { + await github.rest.issues.createLabel({ + owner: context.repo.owner, + repo: context.repo.repo, + name: sizeLabel, + color: labelColor, + }); + } catch (e) { + if (e.status !== 422) throw e; + } + + await github.rest.issues.addLabels({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + labels: [sizeLabel], + }); + + // For large PRs (400+ LOC): auto-apply large-pr label and comment once + if (totalChanges >= 400) { + // Auto-apply the large-pr label + if (!existingLabels.includes('large-pr')) { + try { + await github.rest.issues.createLabel({ + owner: context.repo.owner, + repo: context.repo.repo, + name: 'large-pr', + color: 'e99695', + }); + } catch (e) { + if (e.status !== 422) throw e; + } + + await github.rest.issues.addLabels({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + labels: ['large-pr'], + }); + } + + // Comment once with guidance + const MARKER = ''; + const { data: comments } = await github.rest.issues.listComments({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + }); + + const alreadyCommented = comments.some(c => c.body.includes(MARKER)); + if (!alreadyCommented) { + const prBody = context.payload.pull_request.body || ''; + const guidedTourPresent = /how to review|guided tour|read.*in.*order/i.test(prBody); + + let body = `${MARKER}\n`; + body += `### :straight_ruler: PR Size: **${totalChanges} lines changed** (${sizeLabel})\n\n`; + body += `Please note: this PR exceeds the 400 LOC soft limit.\n`; + body += `- Consider **splitting** into separate PRs if the changes are separable\n`; + body += `- Ensure the PR description includes a **guided tour** in the "How to Review" section so reviewers know where to start\n`; + + if (guidedTourPresent) { + body += `\n:white_check_mark: Guided tour detected — thank you!\n`; + } + + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + body: body, + }); + } + } + + console.log(`PR #${context.issue.number}: ${totalChanges} LOC changed, labeled ${sizeLabel}`); +defaults: + run: + shell: bash -euxo pipefail {0} diff --git a/.github/workflows/stale-pr-reminder.yml b/.github/workflows/stale-pr-reminder.yml new file mode 100644 index 0000000000000000000000000000000000000000..1c3c0aec623c68c3c99803ef2421e73dbec9cf8e --- /dev/null +++ b/.github/workflows/stale-pr-reminder.yml @@ -0,0 +1,115 @@ +# Stale PR Review Reminder +# +# Runs daily on weekdays (second run at 8 PM UTC disabled during rollout) and posts a Slack summary of open PRs that +# have been awaiting review for more than 72 hours. Team-level signal only — +# no individual shaming. +# +# Security note: No untrusted input is interpolated into shell commands. +# All PR metadata is read via gh API + jq. +# +# Required secrets: +# SLACK_WEBHOOK_PR_REVIEW_BOT - Incoming webhook URL for the #pr-review-ops channel + +name: Stale PR Review Reminder + +on: + schedule: + - cron: "0 14 * * 1-5" # 2 PM UTC weekdays + # - cron: "0 20 * * 1-5" # 8 PM UTC weekdays — enable after initial rollout + workflow_dispatch: {} + +permissions: + contents: read + pull-requests: read + +jobs: + check-stale-prs: + if: github.repository_owner == 'zed-industries' + runs-on: ubuntu-latest + timeout-minutes: 5 + env: + REPO: ${{ github.repository }} + # Only surface PRs created on or after this date. Update this if the + # review process enforcement date changes. + PROCESS_START_DATE: "2026-03-19T00:00:00Z" + steps: + - name: Find PRs awaiting review longer than 72h + id: stale + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + CUTOFF=$(date -u -v-72H +%Y-%m-%dT%H:%M:%SZ 2>/dev/null \ + || date -u -d '72 hours ago' +%Y-%m-%dT%H:%M:%SZ) + + # Get open, non-draft PRs with pending review requests, created before cutoff + # but after the review process start date (to exclude pre-existing backlog) + gh api --paginate \ + "repos/${REPO}/pulls?state=open&sort=updated&direction=asc&per_page=100" \ + --jq "[ + .[] | + select(.draft == false) | + select(.created_at > \"$PROCESS_START_DATE\") | + select(.created_at < \"$CUTOFF\") | + select((.requested_reviewers | length > 0) or (.requested_teams | length > 0)) + ]" > /tmp/candidates.json + + # Filter to PRs with zero approving reviews + jq -r '.[].number' /tmp/candidates.json | while read -r PR_NUMBER; do + APPROVALS=$(gh api \ + "repos/${REPO}/pulls/${PR_NUMBER}/reviews" \ + --jq "[.[] | select(.state == \"APPROVED\")] | length" 2>/dev/null || echo "0") + + if [ "$APPROVALS" -eq 0 ]; then + jq ".[] | select(.number == ${PR_NUMBER}) | {number, title, author: .user.login, created_at}" \ + /tmp/candidates.json + fi + done | jq -s '.' > /tmp/awaiting.json + + COUNT=$(jq 'length' /tmp/awaiting.json) + echo "count=$COUNT" >> "$GITHUB_OUTPUT" + + - name: Notify Slack + if: steps.stale.outputs.count != '0' + env: + SLACK_WEBHOOK_PR_REVIEW_BOT: ${{ secrets.SLACK_WEBHOOK_PR_REVIEW_BOT }} + COUNT: ${{ steps.stale.outputs.count }} + run: | + # Build Block Kit payload from JSON — no shell interpolation of PR titles. + # Why jq? PR titles are attacker-controllable input. By reading them + # through jq -r from the JSON file and passing the result to jq --arg, + # the content stays safely JSON-encoded in the final payload. + PRS=$(jq -r '.[] | "• — \(.title) (by \(.author), opened \(.created_at | split("T")[0]))"' /tmp/awaiting.json) + + jq -n \ + --arg count "$COUNT" \ + --arg prs "$PRS" \ + '{ + text: ($count + " PR(s) awaiting review for >72 hours"), + blocks: [ + { + type: "section", + text: { + type: "mrkdwn", + text: (":hourglass_flowing_sand: *" + $count + " PR(s) Awaiting Review >72 Hours*") + } + }, + { + type: "section", + text: { type: "mrkdwn", text: $prs } + }, + { type: "divider" }, + { + type: "context", + elements: [{ + type: "mrkdwn", + text: "PRs awaiting review are surfaced daily. Reviewers: pick one up or reassign." + }] + } + ] + }' | \ + curl -s -X POST "$SLACK_WEBHOOK_PR_REVIEW_BOT" \ + -H 'Content-Type: application/json' \ + -d @- +defaults: + run: + shell: bash -euxo pipefail {0}