Skip to content
73 changes: 7 additions & 66 deletions .github/workflows/test-e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,41 +56,24 @@ jobs:
permissions:
contents: read
steps:
- name: Check if fork PR

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Fork-detection guards removed from test-pirate-agent — dangerous step toward pwn-request if trigger changes

This PR removes the "Check if fork PR" step and all if: steps.fork-check.outputs.is_fork != 'true' conditions from test-pirate-agent. While the current pull_request trigger means fork PRs already receive empty secrets (GitHub's built-in protection), the PR description explicitly states the intent to switch to pull_request_target.

Under pull_request_target + checkout of github.event.pull_request.head.sha + uses: ./, this guard was the only step-level protection against a fork contributor executing their own code with live OPENAI_API_KEY credentials. After this deletion, there is no step-level, job-level, or environment-level gate remaining on this job.

This creates a single-commit path to a critical secret exfiltration vulnerability: applying the stated pull_request_target trigger change without adding new guards would allow any fork PR to run arbitrary action.yml / dist/*.js code with live API keys.

Suggested mitigation: Before or alongside the trigger change, add either:

  1. A job-level guard: if: github.event.pull_request.head.repo.full_name == github.repository || github.event_name != 'pull_request_target'
  2. A GitHub Environment with required reviewers on this job

id: fork-check
run: |
# Use default empty string to handle edge cases (deleted branches, malformed events)
HEAD_REPO="${{ github.event.pull_request.head.repo.full_name || '' }}"
if [[ "${{ github.event_name }}" == "pull_request" && "$HEAD_REPO" != "${{ github.repository }}" && -n "$HEAD_REPO" ]]; then
echo "⏭️ Skipping - fork PR (secrets not available)"
echo "is_fork=true" >> $GITHUB_OUTPUT
else
echo "is_fork=false" >> $GITHUB_OUTPUT
fi

- name: Checkout code
if: steps.fork-check.outputs.is_fork != 'true'
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2

- name: Setup pnpm
if: steps.fork-check.outputs.is_fork != 'true'
uses: pnpm/action-setup@8912a9102ac27614460f54aedde9e1e7f9aec20d # v6.0.5
with:
run_install: false

- name: Setup Node.js
if: steps.fork-check.outputs.is_fork != 'true'
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
with:
node-version: 24
cache: pnpm

- name: Build action
if: steps.fork-check.outputs.is_fork != 'true'
run: pnpm install --frozen-lockfile && pnpm build

- name: Run test
Comment thread
derekmisler marked this conversation as resolved.
if: steps.fork-check.outputs.is_fork != 'true'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] uses: ./ step in test-pirate-agent loses its fork guard — unguarded under pull_request_target

The if: steps.fork-check.outputs.is_fork != 'true' condition is removed from the "Run test" step, which executes the composite action from the local checkout (uses: ./) and passes openai-api-key: ${{ secrets.OPENAI_API_KEY }}.

Today (with pull_request trigger): secrets are empty for fork PRs — no immediate leak.
After the stated trigger change to pull_request_target: a fork contributor controls the checked-out action.yml and dist/*.js files that this step executes, and secrets.OPENAI_API_KEY is a live key.

Note that uses: ./ runs the fork's version of the action — even without the API key, a fork contributor can modify the composite action to leak GITHUB_TOKEN or perform other attacks using the runner's contents: read permission.

id: pirate
uses: ./
with:
Expand All @@ -99,7 +82,6 @@ jobs:
openai-api-key: ${{ secrets.OPENAI_API_KEY }}

- name: Validate output and exit code
if: steps.fork-check.outputs.is_fork != 'true'
run: |
OUTPUT_FILE="${{ steps.pirate.outputs.output-file }}"

Expand Down Expand Up @@ -205,45 +187,27 @@ jobs:
env:
TEST_PR_NUMBER: ${{ github.event.pull_request.number || inputs.pr_number }}
steps:
- name: Check if fork PR

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Fork-detection guards removed from test-mention-reply-toplevel — mitigated today by job-level workflow_dispatch guard, but creates fragile state

The "Check if fork PR" step and all if: steps.fork-check.outputs.is_fork != 'true' step conditions are removed from test-mention-reply-toplevel. The job currently has a job-level if: github.event_name == 'workflow_dispatch' guard, which means fork PRs cannot trigger this job at all today — the removal has no immediate security impact.

However, the same concern applies: if the job-level guard is ever relaxed (e.g., to run on pull_request_target), there would be no step-level fallback. The Setup credentials step (which fetches AWS OIDC-derived OPENAI_API_KEY, ANTHROPIC_API_KEY, GITHUB_APP_TOKEN) would run unconditionally for fork contributors.

The workflow_dispatch guard provides sufficient protection for the current state, but the defense-in-depth layer is gone.

id: fork-check
run: |
# Use default empty string to handle edge cases (deleted branches, malformed events)
HEAD_REPO="${{ github.event.pull_request.head.repo.full_name || '' }}"
if [[ "${{ github.event_name }}" == "pull_request" && "$HEAD_REPO" != "${{ github.repository }}" && -n "$HEAD_REPO" ]]; then
echo "⏭️ Skipping - fork PR (secrets not available)"
echo "is_fork=true" >> $GITHUB_OUTPUT
else
echo "is_fork=false" >> $GITHUB_OUTPUT
fi

- name: Checkout code
if: steps.fork-check.outputs.is_fork != 'true'
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2

- name: Setup pnpm
if: steps.fork-check.outputs.is_fork != 'true'
uses: pnpm/action-setup@8912a9102ac27614460f54aedde9e1e7f9aec20d # v6.0.5
with:
run_install: false

- name: Setup Node.js
if: steps.fork-check.outputs.is_fork != 'true'
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
with:
node-version: 24
cache: pnpm

- name: Build action
if: steps.fork-check.outputs.is_fork != 'true'
run: pnpm install --frozen-lockfile && pnpm build

- name: Setup credentials
if: steps.fork-check.outputs.is_fork != 'true'
uses: docker/cagent-action/setup-credentials@367a30ddb41e0156459d03750f508eac03f3c38a # v1.5.5

- name: Create anchor issue comment on current PR
if: steps.fork-check.outputs.is_fork != 'true'
id: create-anchor
env:
GH_TOKEN: ${{ env.GITHUB_APP_TOKEN || github.token }}
Expand All @@ -256,7 +220,6 @@ jobs:
echo "test_comment_id=$COMMENT_ID" >> $GITHUB_OUTPUT

- name: Write synthetic issue_comment event
if: steps.fork-check.outputs.is_fork != 'true'
run: |
COMMENT_ID="${{ steps.create-anchor.outputs.test_comment_id }}"
jq -n \
Expand All @@ -282,7 +245,6 @@ jobs:
}' > /tmp/test-event-toplevel.json

- name: Run mention-reply handler
if: steps.fork-check.outputs.is_fork != 'true'
id: mention-handler
env:
INPUT_GITHUB-TOKEN: ${{ env.GITHUB_APP_TOKEN || github.token }}
Expand All @@ -293,7 +255,6 @@ jobs:
node "$GITHUB_WORKSPACE/dist/mention-reply.js"

- name: Assert should-reply output
if: steps.fork-check.outputs.is_fork != 'true'
run: |
SHOULD_REPLY="${{ steps.mention-handler.outputs.should-reply }}"
echo "should-reply=$SHOULD_REPLY"
Expand All @@ -307,7 +268,7 @@ jobs:
fi

- name: Run mention reply
if: steps.fork-check.outputs.is_fork != 'true' && steps.mention-handler.outputs.should-reply == 'true'
if: steps.mention-handler.outputs.should-reply == 'true'
id: run-reply
uses: ./review-pr/mention-reply
with:
Expand All @@ -323,7 +284,7 @@ jobs:
skip-auth: "true"

- name: Verify reply was posted
if: steps.fork-check.outputs.is_fork != 'true' && steps.mention-handler.outputs.should-reply == 'true'
if: steps.mention-handler.outputs.should-reply == 'true'
env:
GH_TOKEN: ${{ env.GITHUB_APP_TOKEN || github.token }}
run: |
Expand All @@ -336,7 +297,7 @@ jobs:
echo "✅ Reply posted successfully ($FOUND comment(s) found)"

- name: Cleanup test comments
if: always() && steps.fork-check.outputs.is_fork != 'true'
if: always()
continue-on-error: true
env:
GH_TOKEN: ${{ env.GITHUB_APP_TOKEN || github.token }}
Expand Down Expand Up @@ -366,44 +327,27 @@ jobs:
env:
TEST_PR_NUMBER: ${{ github.event.pull_request.number || inputs.pr_number }}
steps:
- name: Check if fork PR
id: fork-check
run: |
HEAD_REPO="${{ github.event.pull_request.head.repo.full_name || '' }}"
if [[ "${{ github.event_name }}" == "pull_request" && "$HEAD_REPO" != "${{ github.repository }}" && -n "$HEAD_REPO" ]]; then
echo "⏭️ Skipping - fork PR (secrets not available)"
echo "is_fork=true" >> $GITHUB_OUTPUT
else
echo "is_fork=false" >> $GITHUB_OUTPUT
fi

- name: Checkout code
if: steps.fork-check.outputs.is_fork != 'true'
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2

- name: Setup pnpm
if: steps.fork-check.outputs.is_fork != 'true'
uses: pnpm/action-setup@8912a9102ac27614460f54aedde9e1e7f9aec20d # v6.0.5
with:
run_install: false

- name: Setup Node.js
if: steps.fork-check.outputs.is_fork != 'true'
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
with:
node-version: 24
cache: pnpm

- name: Build action
if: steps.fork-check.outputs.is_fork != 'true'
run: pnpm install --frozen-lockfile && pnpm build

- name: Setup credentials
if: steps.fork-check.outputs.is_fork != 'true'
uses: docker/cagent-action/setup-credentials@367a30ddb41e0156459d03750f508eac03f3c38a # v1.5.5

- name: Create anchor review comment on current PR
if: steps.fork-check.outputs.is_fork != 'true'
id: create-anchor
env:
GH_TOKEN: ${{ env.GITHUB_APP_TOKEN || github.token }}
Expand All @@ -428,7 +372,6 @@ jobs:
echo "test_comment_id=$COMMENT_ID" >> $GITHUB_OUTPUT

- name: Write synthetic pull_request_review_comment event
if: steps.fork-check.outputs.is_fork != 'true'
run: |
COMMENT_ID="${{ steps.create-anchor.outputs.test_comment_id }}"
jq -n \
Expand All @@ -455,7 +398,6 @@ jobs:
}' > /tmp/test-event-inline.json

- name: Run mention-reply handler
if: steps.fork-check.outputs.is_fork != 'true'
id: mention-handler
env:
INPUT_GITHUB-TOKEN: ${{ env.GITHUB_APP_TOKEN || github.token }}
Expand All @@ -466,7 +408,6 @@ jobs:
node "$GITHUB_WORKSPACE/dist/mention-reply.js"

- name: Assert should-reply output
if: steps.fork-check.outputs.is_fork != 'true'
run: |
SHOULD_REPLY="${{ steps.mention-handler.outputs.should-reply }}"
echo "should-reply=$SHOULD_REPLY"
Expand All @@ -478,7 +419,7 @@ jobs:
fi

- name: Assert inline outputs
if: steps.fork-check.outputs.is_fork != 'true' && steps.mention-handler.outputs.should-reply == 'true'
if: steps.mention-handler.outputs.should-reply == 'true'
run: |
IS_INLINE="${{ steps.mention-handler.outputs.is-inline }}"
IN_REPLY_TO_ID="${{ steps.mention-handler.outputs.in-reply-to-id }}"
Expand All @@ -495,7 +436,7 @@ jobs:
echo "✅ in-reply-to-id=$IN_REPLY_TO_ID (matches anchor comment)"

- name: Run mention reply
if: steps.fork-check.outputs.is_fork != 'true' && steps.mention-handler.outputs.should-reply == 'true'
if: steps.mention-handler.outputs.should-reply == 'true'
id: run-reply
uses: ./review-pr/mention-reply
with:
Expand All @@ -511,7 +452,7 @@ jobs:
skip-auth: "true"

- name: Verify inline reply was posted in thread
if: steps.fork-check.outputs.is_fork != 'true' && steps.mention-handler.outputs.should-reply == 'true'
if: steps.mention-handler.outputs.should-reply == 'true'
env:
GH_TOKEN: ${{ env.GITHUB_APP_TOKEN || github.token }}
ANCHOR_ID: ${{ steps.create-anchor.outputs.test_comment_id }}
Expand All @@ -526,7 +467,7 @@ jobs:
echo "✅ Inline reply posted successfully"

- name: Cleanup anchor comment and thread replies
if: always() && steps.fork-check.outputs.is_fork != 'true'
if: always()
continue-on-error: true
env:
GH_TOKEN: ${{ env.GITHUB_APP_TOKEN || github.token }}
Expand Down
Loading