Migrate pull request automation away from pull_request_target

[mrecachinas]

Summary

This draft updates the pull request automation in this repository to avoid using pull_request_target for PR-driven workflow execution.

The replacement pattern keeps untrusted PR input in lower-privilege pull_request workflows and moves any required repository-write actions into a separate, narrowly scoped follow-up path. Where a follow-up workflow is needed, it re-checks the pull request context before taking action so the workflow operates on the intended PR/head commit rather than trusting mutable PR state.

Expected workflow shift

• PR-triggered jobs run with reduced permissions.
• Repository write actions, when still needed, happen after the PR workflow completes.
• Follow-up jobs validate PR metadata before posting labels, comments, statuses, or other write-side effects.
• Workflow behavior should remain equivalent for maintainers and contributors, with the permission boundary made more explicit.

Notes

Opening as a draft for repository-owner review before this is marked ready. Please review the workflow-specific behavior and any repository settings assumptions before merge.

[aaronpowell]

Just so I'm properly understanding, it will use a chained PR to run the higher privileged stuff (write comments) rather than run in an arguably less trusted space (PR triggered by someone who wouldn't have permissions).

[mrecachinas]

@aaronpowell That's correct. More details from our Security Lab here: https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/#preventing-pwn-requests.

[aaronpowell]

Awesome - I had missed that advisory.

PR looks good, once it's ready I'll do a final review and merge it in.

[Copilot]

Pull request overview

Migrates PR automation from pull_request_target to lower-privilege pull_request workflows with privileged workflow_run writers.

Changes:

• Produces validated artifacts from PR workflows.
• Adds writers for comments, labels, quality gates, and reputation results.
• Regenerates the duplicate-check workflow with staged outputs.

Show a summary per file

File	Description
pr-duplicate-check.md	Stages duplicate-check output.
pr-duplicate-check.lock.yml	Regenerates the compiled workflow.
pr-duplicate-check-writer.yml	Publishes duplicate comments.
label-pr-intent.yml	Computes label artifacts.
label-pr-intent-writer.yml	Applies intent labels.
external-plugin-pr-quality-gates.yml	Produces quality-result artifacts.
external-plugin-pr-quality-gates-writer.yml	Publishes quality results.
contributor-check.yml	Separates issue and PR checks.
contributor-check-writer.yml	Publishes contributor risk results.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Suppressed comments (3)

.github/workflows/external-plugin-pr-quality-gates-writer.yml:62

• This privileged path trusts the artifact to select the PR without checking that the PR belongs to the triggering workflow_run. A malicious PR artifact can copy another PR's public number/head/base metadata and make this workflow change that PR's intake labels and status comment. Bind payload.pr_number to context.payload.workflow_run.pull_requests and fail closed if no matching association exists.

            if (payload.schema_version !== 'external-plugin-pr-quality-result/v1') fail('unexpected schema_version');
            if (payload.event !== 'pull_request') fail('unexpected event');
            if (!Number.isInteger(payload.pr_number) || payload.pr_number < 1) fail('invalid pr_number');
            if (!/^[0-9a-f]{40}$/i.test(String(payload.head_sha || ''))) fail('invalid head_sha');
            if (!/^[0-9a-f]{40}$/i.test(String(payload.base_sha || ''))) fail('invalid base_sha');
            if (payload.base_ref !== 'main') fail('unexpected base_ref');

.github/workflows/contributor-check-writer.yml:55

• Restricting these values to known risk strings does not establish that AGT produced them. The artifact comes from a PR-controlled workflow, so a contributor can emit LOW/NONE; this writer will then delete an existing warning comment and high-risk label on that same PR. The privileged path must independently verify or recompute the reputation result from trusted default-branch code rather than treating artifact risk values as authoritative.

            for (const key of ['profile_risk', 'credential_risk', 'overall_risk']) {
              if (!allowedRisks.has(result[key])) fail(`invalid ${key}`);
            }

.github/workflows/external-plugin-pr-quality-gates-writer.yml:139

• quality_result_json is accepted as authoritative after only checking that it parses to an object. A PR-controlled run can forge overall_status: "pass" and related fields, causing this privileged writer to apply ready-for-review and publish a passing report without the quality gates succeeding. Recompute the gate result in a trusted path or verify it through a trusted, non-PR-controlled attestation before changing repository state.

              } else if (payload.quality_result_json) {
                qualityResult = JSON.parse(payload.quality_result_json);
                if (!qualityResult || typeof qualityResult !== 'object' || Array.isArray(qualityResult)) {
                  fail('quality_result_json did not parse to an object');
                }

• Files reviewed: 9/9 changed files
• Comments generated: 5
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (5)

.github/workflows/pr-duplicate-check-writer.yml:184

• The replacement sanitizer leaves HTML comments intact. Since this body comes from staged agent output influenced by PR content, it can include another workflow's marker (for example <!-- agt-contributor-check -->); those writers search bot-authored comments by marker and may then update or delete this comment. Neutralize HTML comment delimiters before publishing.

            const sanitizeCommentBody = (value, maxLength) => {
              const sanitized = neutralizeMentions(stripControlCharacters(value));
              return sanitized.length > maxLength ? sanitized.slice(0, maxLength) : sanitized;
            };

.github/workflows/pr-duplicate-check.md:4

• This trigger no longer fires while a PR has merge conflicts, whereas pull_request_target did. That leaves conflicted PRs without duplicate checks and contradicts the stated behavior-equivalence goal. If conflicted PRs remain in scope, provide a trusted metadata-only fallback trigger; otherwise document this limitation explicitly.

  pull_request:

.github/workflows/label-pr-intent.yml:4

• GitHub does not dispatch pull_request workflows while a PR has merge conflicts, unlike pull_request_target. Consequently conflicted PRs will retain stale or missing intent labels, which is not behavior-equivalent. Add a trusted metadata-only fallback path or explicitly accept and document this gap.

  pull_request:

.github/workflows/contributor-check.yml:4

• The contributor check now stops running whenever a PR has merge conflicts, because GitHub suppresses pull_request workflows in that state while pull_request_target still ran. This leaves risk labels/comments stale or absent and breaks the claimed behavior equivalence. Add a trusted fallback path or explicitly document the limitation.

  pull_request:

.github/workflows/external-plugin-pr-quality-gates.yml:4

• Changing this to pull_request means the quality gates do not run for PRs with merge conflicts; pull_request_target did. Such PRs can keep stale quality labels/comments, contrary to the stated behavior-equivalence requirement. Provide a safe fallback trigger or document this as an accepted limitation.

  pull_request:

• Files reviewed: 9/9 changed files
• Comments generated: 0 new
• Review effort level: Balanced