Skip to content

Harden pull request source branch check - #608

Merged
ExtremeFiretop merged 1 commit into
ExtremeFiretop:devfrom
maghuro:security/harden-pr-source-check
Sep 24, 2026
Merged

ExtremeFiretop merged 1 commit into
ExtremeFiretop:devfrom
maghuro:security/harden-pr-source-check

Conversation

@maghuro

@maghuro maghuro commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Harden the pull request source-branch validation workflow while preserving automatic execution for pull requests from forks.

Problem

The current workflow interpolates github.head_ref and github.base_ref directly into an inline shell script.

Pull request branch names are untrusted input and should not be interpreted by the shell.

The workflow also does not require any GITHUB_TOKEN permissions.

Fix

This PR makes two focused changes.

  1. Disable GITHUB_TOKEN permissions.

The workflow performs no GitHub API operations, so it uses:

permissions: {}
  1. Move the branch decision into the GitHub Actions if expression.

Instead of inserting branch names into shell code, the rejection condition is evaluated directly by GitHub Actions:

github.base_ref == 'main' && github.head_ref != 'dev'

The shell step itself now contains only a static error message and exit code.

Why pull_request_target is retained

An earlier revision changed the event to pull_request.

Testing the actual pull request from a fork showed that GitHub placed the workflow in action_required state and required maintainer approval before execution.

That changes the existing behavior and could prevent the source-branch policy from being checked automatically for fork pull requests.

For that reason, pull_request_target is intentionally retained.

The workflow does not check out or execute pull request code, uses no secrets, has no token permissions, and no longer passes pull request branch names into the shell.

Behavior

The existing policy is preserved.

  • PR to main from dev: allowed
  • PR to main from any other branch: rejected
  • PR to dev: allowed
  • PR to another branch: allowed

Scope

This PR changes only .github/workflows/Check-PRsource.yml.

It does not change the repository branch policy itself.

@maghuro
maghuro force-pushed the security/harden-pr-source-check branch from 1004364 to b4226ce Compare September 23, 2026 22:47
@ExtremeFiretop

ExtremeFiretop commented Sep 24, 2026 •

Copy link
Copy Markdown
Owner

I'm merging this one in as well.

It's a "moderate"? GitHub Actions security issue where externally controllable branch names can cause arbitrary command execution on a pull request runner.

But since the the current workflow does not expose secrets or the GITHUB_TOKEN to its shell step, it's substantially limiting the potential impact already.

An attacker could potentially make network requests, read runner-accessible data, consume GitHub-hosted runner resources, or manipulate the branch-check job itself.

However, I don't see a demonstrated path from this workflow to repository takeover, token theft, pushing commits, or modifying MerlinAU source code.

So the injection itself is real and worth fixing, but the current impact appears fairly limited.

@ExtremeFiretop
ExtremeFiretop merged commit 0f0104e into ExtremeFiretop:dev Sep 24, 2026
2 checks passed
@ExtremeFiretop

ExtremeFiretop commented Sep 24, 2026 •

Copy link
Copy Markdown
Owner

I cherry-picked and pushed this PR straight to main.
That way we start now, instead of what the next dev release comes out.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants