Skip to content

[Security Research] Artifact Poisoning PoC — Non-destructive callback in fork-controlled script#169766

Open
rawnly25 wants to merge 6 commits intocockroachdb:masterfrom
rawnly25:security-poc-artifact-poisoning
Open

[Security Research] Artifact Poisoning PoC — Non-destructive callback in fork-controlled script#169766
rawnly25 wants to merge 6 commits intocockroachdb:masterfrom
rawnly25:security-poc-artifact-poisoning

Conversation

@rawnly25
Copy link
Copy Markdown

@rawnly25 rawnly25 commented May 5, 2026

Security Research: Artifact Poisoning Proof of Concept

This PR demonstrates a non-destructive security proof of concept for the workflow_run trust boundary vulnerability in code-cover-publish.yaml.

What this PR does

Adds a harmless curl callback (sending only hostname + whoami) to build/ghactions/pr-codecov-run-tests.sh to prove that fork-controlled scripts execute in the CI context when triggered by code-cover-gen.yml.

Why this matters

The code-cover-publish.yaml workflow:

  • Triggers on workflow_run from any successful pull_request workflow
  • Has no authorization check on the PR source (no label, author, or fork validation)
  • Downloads the artifact from the untrusted PR
  • Authenticates to GCP with a static service account key (secrets.CODECOVER_SERVICE_ACCOUNT_KEY)
  • Processes the artifact data with GCP credentials active

Impact

An attacker can:

  1. Modify fork scripts → execute arbitrary code in the trigger context
  2. Poison the artifact content → influence behavior in the privileged handler
  3. The handler has no source validation → runs with GCP credentials for any PR

This PoC is non-destructive

  • Only sends hostname, whoami, workflow, run_idno secrets or tokens
  • Does not modify any production data
  • Purpose: prove execution context before filing a security report

Suggested Fix

Add source validation to code-cover-publish.yaml:

- name: Verify PR source
  run: |
    if [[ "${{ github.event.workflow_run.head_repository.full_name }}" != "cockroachdb/cockroach" ]]; then
      echo "Artifact from untrusted fork, skipping"
      exit 1
    fi

And migrate from static service account key to OIDC (Workload Identity Federation).

This commit demonstrates that fork-controlled scripts are executed in the workflow_run privileged context. The injected callback only sends hostname/whoami - no secrets are accessed.
@rawnly25 rawnly25 requested a review from a team as a code owner May 5, 2026 19:57
@trunk-io
Copy link
Copy Markdown
Contributor

trunk-io Bot commented May 5, 2026

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented May 5, 2026

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

Before a member of our team reviews your PR, I have some potential action items for you:

  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@blathers-crl blathers-crl Bot added the O-community Originated from the community label May 5, 2026
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@cockroachlabs-cla-agent
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

This ensures code-cover-gen.yml succeeds so code-cover-publish.yaml triggers.
@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented May 5, 2026

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

  • We notice you have more than one commit in your PR. We try break logical changes into separate commits, but commits such as "fix typo" or "address review commits" should be squashed into one commit and pushed with --force
  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented May 5, 2026

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

  • We notice you have more than one commit in your PR. We try break logical changes into separate commits, but commits such as "fix typo" or "address review commits" should be squashed into one commit and pushed with --force
  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

This ensures: 1) Callback fires before any failure, 2) Artifact uploads, 3) Job succeeds so handler triggers
@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented May 5, 2026

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

  • We notice you have more than one commit in your PR. We try break logical changes into separate commits, but commits such as "fix typo" or "address review commits" should be squashed into one commit and pushed with --force
  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

- Fix continue-on-error placement
- Hardcode webhook URL
- Add always() on critical steps
@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented May 5, 2026

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

  • We notice you have more than one commit in your PR. We try break logical changes into separate commits, but commits such as "fix typo" or "address review commits" should be squashed into one commit and pushed with --force
  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented May 5, 2026

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

  • We notice you have more than one commit in your PR. We try break logical changes into separate commits, but commits such as "fix typo" or "address review commits" should be squashed into one commit and pushed with --force
  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

O-community Originated from the community

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants