Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
b628abb
πŸ”’ Fix potential argument injection via PR number string coercion
seonghobae Jun 16, 2026
786eb69
No code changes required for bot command
seonghobae Jun 16, 2026
f952fe4
Fix strict PR number validation: reject bools, floats, and non-digit …
Copilot Jun 16, 2026
6b9edac
Use pr[\"number\"] for clearer KeyError on missing key
Copilot Jun 16, 2026
680e119
No action required for comment directed at other bot
seonghobae Jun 16, 2026
fc2ac4a
Merge remote-tracking branch 'origin/develop' into fix/gh-cli-pr-inje…
Copilot Jun 16, 2026
89d9d05
Merge develop into PR branch and reapply strict _parse_pr_number vali…
Copilot Jun 16, 2026
d627951
No action required for comment directed at other bot
seonghobae Jun 16, 2026
2ad6524
No action required for comment directed at other bot
seonghobae Jun 16, 2026
048716b
Merge remote-tracking branch 'origin/develop' into fix/gh-cli-pr-inje…
Copilot Jun 16, 2026
ed05987
No action required
seonghobae Jun 16, 2026
b85e5f7
Fix strict PR number validation: add _parse_pr_number helper rejectin…
Copilot Jun 16, 2026
39acb3f
No action required for comments directed at other bots
seonghobae Jun 16, 2026
e078c67
No action required
seonghobae Jun 16, 2026
5c58367
Merge origin/develop: Python 3.14 setup and new test files
Copilot Jun 16, 2026
220983d
Fix strict PR number validation: add _parse_pr_number helper rejectin…
Copilot Jun 16, 2026
c4390cb
Merge origin/develop: scanner error path coverage tests
Copilot Jun 16, 2026
72db4b8
No action required
seonghobae Jun 16, 2026
2af9621
Merge remote-tracking branch 'origin/fix/gh-cli-pr-injection-27510907…
Copilot Jun 16, 2026
74c80e7
Fix OpenCode/Strix failure handling: restore _parse_pr_number, Python…
Copilot Jun 16, 2026
5bb52f4
No action required for comments directed at other bots
seonghobae Jun 16, 2026
fe7c108
Merge origin/develop: bring in split_repo tests
Copilot Jun 16, 2026
ec22d2b
Fix strict PR number validation, github-actions[bot] REQUEST_CHANGES …
Copilot Jun 16, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/workflows/opencode-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ jobs:
persist-credentials: true
ref: ${{ github.event.pull_request.head.sha || inputs.pr_head_sha || github.sha }}

- name: Set up Python 3.14
uses: actions/setup-python@v5
with:
python-version: "3.14"
Comment on lines +48 to +51

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify mutable tag references remain in workflows
rg -n 'uses:\s*actions/setup-python@v[0-9]+' .github/workflows

Repository: Seongho-Bae/VibeSec

Length of output: 231


actions/setup-python을 컀밋 SHA둜 κ³ μ •ν•˜μ„Έμš”.

ν˜„μž¬ @v5λŠ” κ°€λ³€ νƒœκ·Έλ‘œ, 곡급망 λ³€μ‘° μ‹œ μ›Œν¬ν”Œλ‘œμ—μ„œ μž„μ˜ μ½”λ“œκ°€ 싀행될 수 μžˆμŠ΅λ‹ˆλ‹€. GitHub Actions λ³΄μ•ˆ λͺ¨λ²” 사둀에 따라 전체 길이의 immutable commit SHA둜 ν•€ 고정이 ν•„μš”ν•©λ‹ˆλ‹€.

πŸ”’ μ œμ•ˆ λ³€κ²½
      - name: Set up Python 3.14
-       uses: actions/setup-python@v5
+       uses: actions/setup-python@0b4efcb60a2b6d2e2e8f3e7e6c5b4a3a2a1a0a1 # v5.x.y
         with:
           python-version: "3.14"
πŸ“ Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Set up Python 3.14
uses: actions/setup-python@v5
with:
python-version: "3.14"
- name: Set up Python 3.14
uses: actions/setup-python@0b4efcb60a2b6d2e2e8f3e7e6c5b4a3a2a1a0a1 # v5.x.y
with:
python-version: "3.14"
🧰 Tools
πŸͺ› zizmor (1.25.2)

[error] 49-49: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

πŸ€– Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/opencode-review.yml around lines 48 - 51, The
`actions/setup-python@v5` action is pinned to a mutable version tag rather than
an immutable commit SHA, creating a supply chain security vulnerability. Replace
the `@v5` version reference in the Set up Python 3.14 step with the full-length
commit SHA of the desired version to ensure the action cannot be tampered with.
Find the appropriate commit SHA from the actions/setup-python repository
releases and update the uses field to include the complete SHA hash.

Source: Linters/SAST tools


- name: Fetch PR base branch for OpenCode context
env:
PR_BASE_REF: ${{ github.event.pull_request.base.ref || inputs.pr_base_ref }}
Expand Down
5 changes: 5 additions & 0 deletions .github/workflows/pr-review-merge-scheduler.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ jobs:
with:
fetch-depth: 1

- name: Set up Python 3.14
uses: actions/setup-python@v5
with:
python-version: "3.14"
Comment on lines +52 to +55

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify mutable tag references remain in workflows
rg -n 'uses:\s*actions/setup-python@v[0-9]+' .github/workflows

Repository: Seongho-Bae/VibeSec

Length of output: 231


actions/setup-python을 κ³ μ • SHA둜 ν•€ν•˜μ‹­μ‹œμ˜€.

@v5 같은 κ°€λ³€ νƒœκ·ΈλŠ” λͺ…μ‹œμ  버전 관리 없이 μ—…λ°μ΄νŠΈλ  수 μžˆμ–΄ 곡급망 λ³΄μ•ˆ μœ„ν—˜μ„ μ΄ˆλž˜ν•©λ‹ˆλ‹€. νŠΉμ • 컀밋 SHA둜 κ³ μ •ν•˜κ±°λ‚˜ νƒœκ·Έ+SHA 쑰합을 μ‚¬μš©ν•΄μ•Ό ν•©λ‹ˆλ‹€.

🧰 Tools
πŸͺ› zizmor (1.25.2)

[error] 53-53: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

πŸ€– Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/pr-review-merge-scheduler.yml around lines 52 - 55, The
actions/setup-python action is pinned to a variable tag (`@v5`) which poses a
supply chain security risk. Pin this action to a specific commit SHA instead of
using the variable tag. Locate the "Set up Python 3.14" step and replace the
`uses: actions/setup-python@v5` with the fully qualified reference that includes
both the version tag and the commit SHA in the format
`actions/setup-python@v5@<commit-sha>` or directly use the SHA hash. This
ensures the workflow uses a fixed, immutable version of the action and prevents
unexpected updates.

Source: Linters/SAST tools


- name: Self-test scheduler
run: python3 scripts/ci/pr_review_merge_scheduler.py --self-test

Expand Down
40 changes: 33 additions & 7 deletions scripts/ci/pr_review_merge_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,35 @@ def review_author_login(review: dict[str, Any]) -> str:
return ((review.get("author") or {}).get("login") or "").lower()


def current_head_review_state(pr: dict[str, Any], state: str) -> bool:
def _parse_pr_number(raw: Any) -> str:
"""Strictly validate and convert a PR number to a positive-integer string.

Accepts only a plain ``int`` (excluding ``bool``) or a digit-only ``str``.
Raises ``ValueError`` for booleans, floats, non-digit strings, zero, and
negative values so that the validated string is safe to pass to the ``gh``
CLI without risk of option/argument injection.
"""
if isinstance(raw, bool):
raise ValueError(f"Invalid PR number (bool not accepted): {raw!r}")
if isinstance(raw, int):
if raw <= 0:
raise ValueError(f"Invalid PR number (must be > 0): {raw!r}")
return str(raw)
if isinstance(raw, str):
if not raw.isdigit():
raise ValueError(f"Invalid PR number (non-digit string): {raw!r}")
value = int(raw)
if value <= 0:
raise ValueError(f"Invalid PR number (must be > 0): {raw!r}")
return str(value)
raise ValueError(f"Invalid PR number (unexpected type {type(raw).__name__}): {raw!r}")


def current_head_review_state(pr: dict[str, Any], state: str, *, extra_authors: tuple[str, ...] = ()) -> bool:
head = pr.get("headRefOid")
for review in reversed((pr.get("reviews") or {}).get("nodes") or []):
if not review_author_login(review).startswith("opencode-agent"):
login = review_author_login(review)
if not (login.startswith("opencode-agent") or login in extra_authors):
continue
if (review.get("state") or "").upper() != state:
continue
Expand All @@ -177,18 +202,19 @@ def has_current_head_approval(pr: dict[str, Any]) -> bool:


def has_current_head_changes_requested(pr: dict[str, Any]) -> bool:
return current_head_review_state(pr, "CHANGES_REQUESTED")
return current_head_review_state(pr, "CHANGES_REQUESTED", extra_authors=("github-actions[bot]",))


def enable_auto_merge(repo: str, pr: dict[str, Any], *, dry_run: bool) -> None:
number = str(pr["number"])
head = pr["headRefOid"]
number = _parse_pr_number(pr["number"])
head = str(pr["headRefOid"])
if dry_run:
return
run(["gh", "pr", "merge", number, "--repo", repo, "--auto", "--merge", "--match-head-commit", head])


def dispatch_opencode_review(repo: str, workflow: str, pr: dict[str, Any], *, dry_run: bool) -> None:
number = _parse_pr_number(pr["number"])
if dry_run:
return
run(
Expand All @@ -202,7 +228,7 @@ def dispatch_opencode_review(repo: str, workflow: str, pr: dict[str, Any], *, dr
"--ref",
pr["baseRefName"],
"-f",
f"pr_number={pr['number']}",
f"pr_number={number}",
"-f",
f"pr_base_ref={pr['baseRefName']}",
"-f",
Expand All @@ -224,7 +250,7 @@ def inspect_pr(
enable_auto_merge_flag: bool,
workflow: str,
) -> Decision:
number = pr["number"]
number = int(_parse_pr_number(pr["number"]))
head_repo = (pr.get("headRepository") or {}).get("nameWithOwner")

if pr.get("isDraft"):
Expand Down
83 changes: 82 additions & 1 deletion tests/scripts/ci/test_pr_review_merge_scheduler.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import pytest
from scripts.ci.pr_review_merge_scheduler import split_repo
from scripts.ci.pr_review_merge_scheduler import (
_parse_pr_number,
has_current_head_approval,
has_current_head_changes_requested,
split_repo,
)

def test_split_repo_valid():
assert split_repo("owner/name") == ("owner", "name")
Expand All @@ -17,3 +22,79 @@ def test_split_repo_invalid():

with pytest.raises(ValueError, match="repo must be owner/name, got '/'"):
split_repo("/")


# --- _parse_pr_number tests ---

def test_parse_pr_number_valid_int():
assert _parse_pr_number(1) == "1"
assert _parse_pr_number(42) == "42"

def test_parse_pr_number_valid_str():
assert _parse_pr_number("5") == "5"
assert _parse_pr_number("100") == "100"

def test_parse_pr_number_rejects_bool():
with pytest.raises(ValueError, match="bool not accepted"):
_parse_pr_number(True)
with pytest.raises(ValueError, match="bool not accepted"):
_parse_pr_number(False)

def test_parse_pr_number_rejects_float():
with pytest.raises(ValueError, match="unexpected type float"):
_parse_pr_number(1.9)

def test_parse_pr_number_rejects_non_digit_string():
with pytest.raises(ValueError, match="non-digit string"):
_parse_pr_number("abc")
with pytest.raises(ValueError, match="non-digit string"):
_parse_pr_number("1.9")
with pytest.raises(ValueError, match="non-digit string"):
_parse_pr_number("-1")

def test_parse_pr_number_rejects_zero():
with pytest.raises(ValueError, match="must be > 0"):
_parse_pr_number(0)
with pytest.raises(ValueError, match="must be > 0"):
_parse_pr_number("0")

def test_parse_pr_number_rejects_negative():
with pytest.raises(ValueError, match="must be > 0"):
_parse_pr_number(-5)

def test_parse_pr_number_rejects_none():
with pytest.raises(ValueError, match="unexpected type NoneType"):
_parse_pr_number(None)


# --- has_current_head_changes_requested with github-actions[bot] ---

def _make_pr(head_oid: str, reviews: list) -> dict:
return {
"headRefOid": head_oid,
"reviewDecision": "CHANGES_REQUESTED",
"reviews": {"nodes": reviews},
"reviewThreads": {"nodes": []},
"statusCheckRollup": {"contexts": {"nodes": []}},
}

def test_github_actions_bot_changes_requested_current_head():
pr = _make_pr("abc", [
{"state": "CHANGES_REQUESTED", "author": {"login": "github-actions[bot]"}, "commit": {"oid": "abc"}}
])
assert has_current_head_changes_requested(pr)

def test_github_actions_bot_changes_requested_old_commit_not_blocking():
pr = _make_pr("newhead", [
{"state": "CHANGES_REQUESTED", "author": {"login": "github-actions[bot]"}, "commit": {"oid": "oldhead"}}
])
assert not has_current_head_changes_requested(pr)

def test_github_actions_bot_approval_not_recognised():
"""github-actions[bot] APPROVED should not trigger auto-merge."""
pr = _make_pr("abc", [
{"state": "APPROVED", "author": {"login": "github-actions[bot]"}, "commit": {"oid": "abc"}}
])
# reviewDecision is CHANGES_REQUESTED so the fallback won't fire either
pr["reviewDecision"] = "REVIEW_REQUIRED"
assert not has_current_head_approval(pr)