You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tooling.apache.org by ak...@apache.org on 2026/04/10 07:32:43 UTC
(tooling-agents) branch main updated: Cleaning up reports and adding tests
This is an automated email from the ASF dual-hosted git repository.
akm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tooling-agents.git
The following commit(s) were added to refs/heads/main by this push:
new 1498936 Cleaning up reports and adding tests
1498936 is described below
commit 14989365d5b4a6cec2eeff26a84060f6d70615f5
Author: Andrew Musselman <an...@gmail.com>
AuthorDate: Fri Apr 10 00:32:30 2026 -0700
Cleaning up reports and adding tests
---
.gitignore | 1 +
repos/apache/github-review/agents/README.md | 15 +++-
repos/apache/github-review/agents/brief.py | 10 +--
repos/apache/github-review/agents/json-export.py | 13 ++-
repos/apache/github-review/agents/review.py | 43 ++++++---
repos/apache/github-review/agents/security.py | 100 ++++++++++++++++-----
repos/apache/github-review/agents/tests/README.md | 19 +++-
.../__pycache__/security_checks.cpython-313.pyc | Bin 16442 -> 0 bytes
.../synthetic/self-hosted-low-both-mitigations.yml | 12 +++
.../synthetic/self-hosted-medium-labeled.yml | 10 +++
.../synthetic/self-hosted-medium-limited-perms.yml | 12 +++
.../github-review/agents/tests/security_checks.py | 91 ++++++++++++++++---
.../agents/tests/test_security_checks.py | 20 ++++-
13 files changed, 286 insertions(+), 60 deletions(-)
diff --git a/.gitignore b/.gitignore
new file mode 100644
index 0000000..bee8a64
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1 @@
+__pycache__
diff --git a/repos/apache/github-review/agents/README.md b/repos/apache/github-review/agents/README.md
index 73f20c3..b76378c 100644
--- a/repos/apache/github-review/agents/README.md
+++ b/repos/apache/github-review/agents/README.md
@@ -132,7 +132,7 @@ The Security agent runs 13 checks. When adding a new check, also update `ATTACK_
| Check | Severity | What It Detects |
|-------|----------|-----------------|
| `prt_checkout` | CRITICAL–LOW | `pull_request_target` + checkout of PR head. Severity uses a 2×2 matrix (see below). |
-| `self_hosted_runner` | HIGH | Self-hosted runners exposed to PR triggers |
+| `self_hosted_runner` | HIGH–LOW | Self-hosted runners exposed to PR triggers. Severity uses same 2×2 matrix as prt_checkout. |
| `unpinned_actions` | MEDIUM | Actions referenced by mutable tags instead of SHA pins |
| `composite_action_unpinned` | MEDIUM | Unpinned actions inside composite actions |
| `composite_action_input_injection` | MEDIUM | Composite action interpolates `inputs.*` in run blocks — latent injection surface |
@@ -160,6 +160,19 @@ When `pull_request_target` checks out PR head code, severity depends on two fact
- Default checkout (no `ref:` parameter) → INFO (safe, checks out base branch)
- Explicit base ref checkout → INFO (safe)
+### self_hosted_runner severity matrix
+
+Same 2×2 as prt_checkout but with HIGH ceiling instead of CRITICAL (runner compromise is serious but doesn't directly grant access to base repo secrets like prt_checkout does):
+
+| | Broad permissions | Limited permissions |
+|------------------------|-------------------|---------------------|
+| **Auto-trigger** (opened, synchronize) | **HIGH** | **MEDIUM** |
+| **Maintainer-gated** (labeled, assigned) | **MEDIUM** | **LOW** |
+
+No PR trigger → INFO (runners used for scheduled/manual workflows only).
+
+Uses the same `extract_permissions()` and generalized `extract_trigger_event_types()` functions as prt_checkout.
+
### run_block_injection trigger awareness
Interpolation of PR-related values (`event.pull_request.title`, `.body`, `.head.ref`, etc.) in `run:` blocks:
diff --git a/repos/apache/github-review/agents/brief.py b/repos/apache/github-review/agents/brief.py
index 87013d1..07e0271 100644
--- a/repos/apache/github-review/agents/brief.py
+++ b/repos/apache/github-review/agents/brief.py
@@ -47,7 +47,7 @@ async def run(input_dict, tools):
if stripped.startswith("|") and "|" in stripped[1:]:
parts = [p.strip() for p in stripped.split("|")]
if len(parts) >= 3 and parts[1] and parts[1] not in ("Repository", ""):
- repo_name = parts[1].strip()
+ repo_name = parts[1].strip().strip("`").strip()
if repo_name and not repo_name.startswith("-"):
if repo_filter is None or repo_name in repo_filter:
tp_repo_set.add(repo_name)
@@ -124,10 +124,10 @@ async def run(input_dict, tools):
print(f"HIGH (non-publishing): {len(high_nonpublishing)} findings in {len(high_nonpub_repos)} repos", flush=True)
# --- Build brief ---
- PUB = "apache-github-publishing.md"
- SEC = "apache-github-security.md"
- REVIEW = "apache-github-review.md"
- JSON = "apache-github-review.json"
+ PUB = "publishing.md"
+ SEC = "security.md"
+ REVIEW = "review.md"
+ JSON = "json-export.json"
def plural(n, singular, plural_form=None):
if plural_form is None:
diff --git a/repos/apache/github-review/agents/json-export.py b/repos/apache/github-review/agents/json-export.py
index 7d02680..6d9915a 100644
--- a/repos/apache/github-review/agents/json-export.py
+++ b/repos/apache/github-review/agents/json-export.py
@@ -293,11 +293,18 @@ async def run(input_dict, tools):
},
"self_hosted_runner": {
"label": "Self-Hosted Runner Exposure",
- "severity": "MEDIUM",
- "description": "Workflow runs on self-hosted runners with PR triggers.",
- "attack": "Self-hosted runners persist state between jobs. An attacker's PR can execute arbitrary code on the runner, install backdoors, or steal credentials cached on disk.",
+ "severity": "HIGH",
+ "description": "Workflow runs on self-hosted runners with PR triggers. Severity depends on permissions and trigger type.",
+ "attack": "Self-hosted runners persist state between jobs. An attacker's PR can execute arbitrary code on the runner, install backdoors, steal credentials cached on disk, or pivot to internal networks.",
"example": "1. Workflow runs on self-hosted runner and triggers on pull_request\n2. Attacker's PR executes: curl http://169.254.169.254/latest/meta-data/\n3. AWS instance credentials are exfiltrated\n4. Attacker gains access to internal infrastructure",
},
+ "missing_dependency_updates": {
+ "label": "No Dependency Update Configuration",
+ "severity": "INFO",
+ "description": "Repository has no dependabot.yml or renovate.json for automated dependency updates.",
+ "attack": "Without automated dependency updates, vulnerable transitive dependencies may persist indefinitely. Actions pinned to SHA are not automatically updated when security fixes are released.",
+ "example": "1. Repository uses actions/checkout@abc123 (pinned to SHA)\n2. A security vulnerability is found in that version\n3. No Dependabot or Renovate config to create update PRs\n4. Vulnerable action version persists until manually updated",
+ },
}
# --- Build top-level summary ---
diff --git a/repos/apache/github-review/agents/review.py b/repos/apache/github-review/agents/review.py
index 5cc0ec8..01e5ec4 100644
--- a/repos/apache/github-review/agents/review.py
+++ b/repos/apache/github-review/agents/review.py
@@ -112,7 +112,7 @@ async def run(input_dict, tools):
if tp_section and "| " in line and "`" in line:
parts = line.split("|")
if len(parts) > 1:
- repo_name = parts[1].strip()
+ repo_name = parts[1].strip().strip("`").strip()
if repo_name and repo_name != "Repository":
tp_repos.add(repo_name)
@@ -164,8 +164,8 @@ async def run(input_dict, tools):
low_repos = [r for r in repo_rows if r["worst"] in ("LOW", "INFO", "—") and r["publishes"]]
# --- Generate report ---
- PUB = "apache-github-publishing.md"
- SEC = "apache-github-security.md"
+ PUB = "publishing.md"
+ SEC = "security.md"
def anchor(text):
a = text.lower().strip()
@@ -245,9 +245,7 @@ async def run(input_dict, tools):
# --- Findings by Vulnerability Type ---
check_counts = sec_stats.get("check_counts", {})
- # Filter out informational-only checks
- info_checks = {"composite_actions_scanned", "missing_dependency_updates"}
- vuln_checks = {k: v for k, v in check_counts.items() if k not in info_checks and v > 0}
+ vuln_checks = {k: v for k, v in check_counts.items() if v > 0}
ATTACK_SCENARIOS = {
"prt_checkout": {
@@ -401,8 +399,8 @@ async def run(input_dict, tools):
},
"self_hosted_runner": {
"label": "Self-Hosted Runner Exposure",
- "severity": "HIGH",
- "description": "Workflow runs on self-hosted runners with PR triggers.",
+ "severity": "HIGH–LOW",
+ "description": "Workflow runs on self-hosted runners with PR triggers. Severity depends on permissions and trigger type.",
"attack": ("Self-hosted runners persist state between jobs. An attacker's PR can execute "
"arbitrary code on the runner, install backdoors, steal credentials cached on disk, "
"or pivot to internal networks the runner has access to."),
@@ -411,6 +409,18 @@ async def run(input_dict, tools):
"3. AWS instance credentials are exfiltrated\n"
"4. Attacker gains access to internal infrastructure"),
},
+ "missing_dependency_updates": {
+ "label": "No Automated Dependency Updates",
+ "severity": "INFO",
+ "description": "No dependabot.yml or renovate.json for automated dependency updates.",
+ "attack": ("Without automated dependency updates, vulnerable transitive dependencies and "
+ "SHA-pinned actions persist indefinitely. Security fixes for actions and libraries "
+ "are not surfaced as PRs, leaving known vulnerabilities unpatched."),
+ "example": ("1. Repository uses `actions/checkout@abc123` (pinned to SHA)\n"
+ "2. A security vulnerability is found in that version\n"
+ "3. No Dependabot or Renovate config to create update PRs\n"
+ "4. Vulnerable action version persists until manually updated"),
+ },
}
if vuln_checks:
@@ -548,20 +558,29 @@ async def run(input_dict, tools):
tp_ecosystems = {}
if pub_report:
+ in_migration_section = False
current_eco = None
for line in pub_report.split("\n"):
+ # Only parse ecosystems within the Migration Opportunities section
+ if "Trusted Publishing Migration" in line and line.startswith("## "):
+ in_migration_section = True
+ continue
+ if not in_migration_section:
+ continue
+ # Stop at the next ## heading that isn't about TP
+ if line.startswith("## ") and "Trusted Publishing" not in line:
+ break
if line.startswith("### ") and "Trusted Publishing" not in line:
eco_name = line[4:].strip()
if eco_name in ("crates.io", "npm", "NuGet", "PyPI", "RubyGems"):
current_eco = eco_name
continue
- if current_eco and line.startswith("## "):
- current_eco = None
- continue
+ else:
+ current_eco = None
if current_eco and "| " in line and "`" in line:
parts = [p.strip() for p in line.split("|")]
if len(parts) > 2 and parts[1] and parts[1] != "Repository":
- tp_ecosystems.setdefault(current_eco, []).append(parts[1])
+ tp_ecosystems.setdefault(current_eco, []).append(parts[1].strip("`"))
for eco, repos in sorted(tp_ecosystems.items()):
unique = sorted(set(repos))
diff --git a/repos/apache/github-review/agents/security.py b/repos/apache/github-review/agents/security.py
index 0998573..fe1f159 100644
--- a/repos/apache/github-review/agents/security.py
+++ b/repos/apache/github-review/agents/security.py
@@ -245,20 +245,21 @@ async def run(input_dict, tools):
return findings
- def extract_prt_event_types(content):
- """Extract event types for pull_request_target (e.g., labeled, opened, synchronize)."""
+ def extract_trigger_event_types(content, trigger_name):
+ """Extract event types for any trigger (e.g., pull_request_target, pull_request).
+ Returns set of types like {'labeled', 'opened', 'synchronize'}."""
types = set()
- in_prt = False
- prt_indent = 0
+ in_trigger = False
+ trigger_indent = 0
for line in content.split("\n"):
stripped = line.strip()
indent = len(line) - len(line.lstrip())
- if stripped.startswith("pull_request_target"):
- in_prt = True
- prt_indent = indent
+ if stripped.startswith(trigger_name):
+ in_trigger = True
+ trigger_indent = indent
continue
- if in_prt:
- if stripped and indent <= prt_indent and not stripped.startswith("types"):
+ if in_trigger:
+ if stripped and indent <= trigger_indent and not stripped.startswith("types"):
break
if stripped.startswith("types:"):
rest = stripped[6:].strip()
@@ -270,6 +271,10 @@ async def run(input_dict, tools):
types.add(stripped.lstrip("- ").strip())
return types
+ def extract_prt_event_types(content):
+ """Extract event types for pull_request_target (e.g., labeled, opened, synchronize)."""
+ return extract_trigger_event_types(content, "pull_request_target")
+
def check_prt_checkout(content):
triggers = extract_triggers(content)
if "pull_request_target" not in triggers:
@@ -403,13 +408,71 @@ async def run(input_dict, tools):
def check_self_hosted(content, triggers):
has_self_hosted = "self-hosted" in content
+ if not has_self_hosted:
+ return None
+
has_pr_trigger = bool(triggers & PR_TRIGGERS)
- if has_self_hosted and has_pr_trigger:
- return ("HIGH", "Self-hosted runner with PR trigger. External contributors can "
- "execute arbitrary code on self-hosted infrastructure.")
- elif has_self_hosted:
+ if not has_pr_trigger:
return ("INFO", "Uses self-hosted runners. Ensure runners are ephemeral.")
- return None
+
+ # --- PR trigger present — assess mitigating factors (same 2x2 as prt_checkout) ---
+
+ # Factor 1: Event types — is this maintainer-gated?
+ maintainer_gated_types = {"labeled", "unlabeled", "assigned", "unassigned",
+ "review_requested", "review_request_removed"}
+ pr_event_types = set()
+ for t in ("pull_request_target", "pull_request"):
+ if t in triggers:
+ pr_event_types |= extract_trigger_event_types(content, t)
+
+ is_maintainer_gated = bool(pr_event_types) and pr_event_types.issubset(maintainer_gated_types)
+
+ # Factor 2: Permissions — what can the workflow actually do?
+ perms = extract_permissions(content)
+ dangerous_perms = {"contents", "packages", "id-token", "actions"}
+ has_dangerous_perms = False
+ perm_level = perms.get("_level", "")
+
+ if perm_level in ("write-all",):
+ has_dangerous_perms = True
+ elif not perms:
+ has_dangerous_perms = True
+ else:
+ for perm_name in dangerous_perms:
+ if perms.get(perm_name) == "write":
+ has_dangerous_perms = True
+ break
+
+ # Build detail with mitigating factors
+ mitigations = []
+ if is_maintainer_gated:
+ mitigations.append(f"trigger restricted to maintainer-gated events ({', '.join(sorted(pr_event_types))})")
+ if not has_dangerous_perms:
+ write_perms = [k for k, v in perms.items() if v == "write" and k != "_level"]
+ mitigations.append(f"limited permissions ({', '.join(write_perms) if write_perms else 'read-only'})")
+
+ # Determine severity (same 2x2 as prt_checkout but HIGH ceiling instead of CRITICAL)
+ if is_maintainer_gated and not has_dangerous_perms:
+ return ("LOW",
+ "Self-hosted runner with PR trigger, but risk is mitigated: "
+ + "; ".join(mitigations) + ". "
+ "Maintainer must trigger the workflow and permissions limit blast radius.")
+ elif is_maintainer_gated:
+ return ("MEDIUM",
+ "Self-hosted runner with PR trigger and dangerous permissions, "
+ "but trigger requires maintainer action ("
+ + ", ".join(sorted(pr_event_types)) + "). "
+ "A maintainer labeling a malicious PR would grant runner access to secrets.")
+ elif not has_dangerous_perms:
+ write_perms = [k for k, v in perms.items() if v == "write" and k != "_level"]
+ return ("MEDIUM",
+ "Self-hosted runner with PR trigger, but permissions are limited to: "
+ + (", ".join(write_perms) if write_perms else "read-only") + ". "
+ "Blast radius is reduced — no access to publishing secrets or contents:write.")
+ else:
+ return ("HIGH",
+ "Self-hosted runner with PR trigger. External contributors can "
+ "execute arbitrary code on self-hosted infrastructure with broad permissions.")
def check_permissions(content):
perms = extract_permissions(content)
@@ -760,12 +823,8 @@ async def run(input_dict, tools):
repo_findings.extend(composite_findings)
if composite_total > 0:
- repo_findings.append({
- "check": "composite_actions_scanned", "severity": "INFO",
- "file": ".github/actions/",
- "detail": (f"{composite_analyzed}/{composite_total} composite actions analyzed. "
- f"{len(composite_findings)} finding(s)."),
- })
+ print(f" Composite actions: {composite_analyzed}/{composite_total} analyzed, "
+ f"{len(composite_findings)} finding(s)", flush=True)
# Deduplicate all findings for this repo
repo_findings = deduplicate_findings(repo_findings)
@@ -819,7 +878,6 @@ async def run(input_dict, tools):
"codeowners_gap": "CODEOWNERS missing .github/ coverage",
"missing_codeowners": "No CODEOWNERS file",
"missing_dependency_updates": "No dependabot/renovate configuration",
- "composite_actions_scanned": "Composite actions analyzed",
"composite_action_injection": "Injection in composite action run block",
"composite_action_unpinned": "Unpinned action ref inside composite action",
"composite_action_input_injection": "Latent injection surface — composite action interpolates inputs.* in run block",
diff --git a/repos/apache/github-review/agents/tests/README.md b/repos/apache/github-review/agents/tests/README.md
index f8c663d..1a5a656 100644
--- a/repos/apache/github-review/agents/tests/README.md
+++ b/repos/apache/github-review/agents/tests/README.md
@@ -22,7 +22,7 @@ Running 16 tests...
PASS REAL: Texera email notif — injection LOW (pull_request trigger, not prt)
============================================================
-Results: 16 passed, 0 failed, 0 errors
+Results: 19 passed, 0 failed, 0 errors
============================================================
```
@@ -39,13 +39,23 @@ The `pull_request_target` + checkout check uses a 2×2 severity matrix based on
Additional cases: INFO for default ref (safe), None when no prt trigger.
+### self_hosted_runner — severity matrix (5 tests)
+
+Same 2×2 pattern as prt_checkout but with HIGH ceiling (runner compromise is serious but doesn't directly grant base repo secrets like prt_checkout):
+
+| | Broad permissions | Limited permissions |
+|------------------------|-------------------|---------------------|
+| **Auto-trigger** | HIGH | MEDIUM |
+| **Maintainer-gated** | MEDIUM | LOW |
+
+Additional case: INFO for push-only (no PR trigger).
+
### run_block_injection — trigger-aware (3 tests)
Interpolation of untrusted values (`event.pull_request.title`, etc.) in `run:` blocks. Severity depends on whether the trigger is `pull_request` (fork PRs don't get secrets → LOW) or `pull_request_target` (fork PRs DO get secrets → CRITICAL). Secrets interpolation is always LOW.
-### Other checks (4 tests)
+### Other checks (2 tests)
-- `self_hosted`: HIGH with PR trigger, INFO without
- `cache_poisoning`: INFO when actions/cache + PR trigger
- `permissions`: HIGH for write-all
@@ -78,6 +88,9 @@ tests/
│ ├── injection-low-pr-trigger.yml
│ ├── injection-low-secret.yml
│ ├── self-hosted-high-pr.yml
+ │ ├── self-hosted-medium-limited-perms.yml
+ │ ├── self-hosted-medium-labeled.yml
+ │ ├── self-hosted-low-both-mitigations.yml
│ ├── self-hosted-info-push.yml
│ ├── cache-poisoning-info.yml
│ └── broad-perms-high.yml
diff --git a/repos/apache/github-review/agents/tests/__pycache__/security_checks.cpython-313.pyc b/repos/apache/github-review/agents/tests/__pycache__/security_checks.cpython-313.pyc
deleted file mode 100644
index 6086cd8..0000000
Binary files a/repos/apache/github-review/agents/tests/__pycache__/security_checks.cpython-313.pyc and /dev/null differ
diff --git a/repos/apache/github-review/agents/tests/fixtures/synthetic/self-hosted-low-both-mitigations.yml b/repos/apache/github-review/agents/tests/fixtures/synthetic/self-hosted-low-both-mitigations.yml
new file mode 100644
index 0000000..c4a7e95
--- /dev/null
+++ b/repos/apache/github-review/agents/tests/fixtures/synthetic/self-hosted-low-both-mitigations.yml
@@ -0,0 +1,12 @@
+name: Test on self-hosted with both mitigations
+on:
+ pull_request_target:
+ types: [labeled, assigned]
+permissions:
+ pull-requests: write
+jobs:
+ test:
+ runs-on: self-hosted
+ steps:
+ - uses: actions/checkout@v4
+ - run: make test
diff --git a/repos/apache/github-review/agents/tests/fixtures/synthetic/self-hosted-medium-labeled.yml b/repos/apache/github-review/agents/tests/fixtures/synthetic/self-hosted-medium-labeled.yml
new file mode 100644
index 0000000..6c8cc58
--- /dev/null
+++ b/repos/apache/github-review/agents/tests/fixtures/synthetic/self-hosted-medium-labeled.yml
@@ -0,0 +1,10 @@
+name: Test on self-hosted with labeled trigger
+on:
+ pull_request_target:
+ types: [labeled]
+jobs:
+ test:
+ runs-on: self-hosted
+ steps:
+ - uses: actions/checkout@v4
+ - run: make test
diff --git a/repos/apache/github-review/agents/tests/fixtures/synthetic/self-hosted-medium-limited-perms.yml b/repos/apache/github-review/agents/tests/fixtures/synthetic/self-hosted-medium-limited-perms.yml
new file mode 100644
index 0000000..491c68f
--- /dev/null
+++ b/repos/apache/github-review/agents/tests/fixtures/synthetic/self-hosted-medium-limited-perms.yml
@@ -0,0 +1,12 @@
+name: Test on self-hosted with limited perms
+on:
+ pull_request:
+permissions:
+ pull-requests: write
+ checks: write
+jobs:
+ test:
+ runs-on: self-hosted
+ steps:
+ - uses: actions/checkout@v4
+ - run: make test
diff --git a/repos/apache/github-review/agents/tests/security_checks.py b/repos/apache/github-review/agents/tests/security_checks.py
index 6fe79b3..2529c51 100644
--- a/repos/apache/github-review/agents/tests/security_checks.py
+++ b/repos/apache/github-review/agents/tests/security_checks.py
@@ -69,20 +69,20 @@ def extract_permissions(content):
return perms
-def extract_prt_event_types(content):
- """Extract event types for pull_request_target (e.g., labeled, opened, synchronize)."""
+def extract_trigger_event_types(content, trigger_name):
+ """Extract event types for any trigger (e.g., pull_request_target, pull_request)."""
types = set()
- in_prt = False
- prt_indent = 0
+ in_trigger = False
+ trigger_indent = 0
for line in content.split("\n"):
stripped = line.strip()
indent = len(line) - len(line.lstrip())
- if stripped.startswith("pull_request_target"):
- in_prt = True
- prt_indent = indent
+ if stripped.startswith(trigger_name):
+ in_trigger = True
+ trigger_indent = indent
continue
- if in_prt:
- if stripped and indent <= prt_indent and not stripped.startswith("types"):
+ if in_trigger:
+ if stripped and indent <= trigger_indent and not stripped.startswith("types"):
break
if stripped.startswith("types:"):
rest = stripped[6:].strip()
@@ -95,6 +95,11 @@ def extract_prt_event_types(content):
return types
+def extract_prt_event_types(content):
+ """Extract event types for pull_request_target (e.g., labeled, opened, synchronize)."""
+ return extract_trigger_event_types(content, "pull_request_target")
+
+
def _classify_interpolation(line, step_name, context_label="", triggers=None):
findings = []
prefix = f" in {context_label}" if context_label else ""
@@ -318,13 +323,71 @@ def check_self_hosted(content, triggers=None):
if triggers is None:
triggers = extract_triggers(content)
has_self_hosted = "self-hosted" in content
+ if not has_self_hosted:
+ return None
+
has_pr_trigger = bool(triggers & PR_TRIGGERS)
- if has_self_hosted and has_pr_trigger:
- return ("HIGH", "Self-hosted runner with PR trigger. External contributors can "
- "execute arbitrary code on self-hosted infrastructure.")
- elif has_self_hosted:
+ if not has_pr_trigger:
return ("INFO", "Uses self-hosted runners. Ensure runners are ephemeral.")
- return None
+
+ # --- PR trigger present — assess mitigating factors (same 2x2 as prt_checkout) ---
+
+ # Factor 1: Event types — is this maintainer-gated?
+ maintainer_gated_types = {"labeled", "unlabeled", "assigned", "unassigned",
+ "review_requested", "review_request_removed"}
+ pr_event_types = set()
+ for t in ("pull_request_target", "pull_request"):
+ if t in triggers:
+ pr_event_types |= extract_trigger_event_types(content, t)
+
+ is_maintainer_gated = bool(pr_event_types) and pr_event_types.issubset(maintainer_gated_types)
+
+ # Factor 2: Permissions — what can the workflow actually do?
+ perms = extract_permissions(content)
+ dangerous_perms = {"contents", "packages", "id-token", "actions"}
+ has_dangerous_perms = False
+ perm_level = perms.get("_level", "")
+
+ if perm_level in ("write-all",):
+ has_dangerous_perms = True
+ elif not perms:
+ has_dangerous_perms = True
+ else:
+ for perm_name in dangerous_perms:
+ if perms.get(perm_name) == "write":
+ has_dangerous_perms = True
+ break
+
+ # Build detail with mitigating factors
+ mitigations = []
+ if is_maintainer_gated:
+ mitigations.append(f"trigger restricted to maintainer-gated events ({', '.join(sorted(pr_event_types))})")
+ if not has_dangerous_perms:
+ write_perms = [k for k, v in perms.items() if v == "write" and k != "_level"]
+ mitigations.append(f"limited permissions ({', '.join(write_perms) if write_perms else 'read-only'})")
+
+ # Determine severity (same 2x2 as prt_checkout but HIGH ceiling instead of CRITICAL)
+ if is_maintainer_gated and not has_dangerous_perms:
+ return ("LOW",
+ "Self-hosted runner with PR trigger, but risk is mitigated: "
+ + "; ".join(mitigations) + ". "
+ "Maintainer must trigger the workflow and permissions limit blast radius.")
+ elif is_maintainer_gated:
+ return ("MEDIUM",
+ "Self-hosted runner with PR trigger and dangerous permissions, "
+ "but trigger requires maintainer action ("
+ + ", ".join(sorted(pr_event_types)) + "). "
+ "A maintainer labeling a malicious PR would grant runner access to secrets.")
+ elif not has_dangerous_perms:
+ write_perms = [k for k, v in perms.items() if v == "write" and k != "_level"]
+ return ("MEDIUM",
+ "Self-hosted runner with PR trigger, but permissions are limited to: "
+ + (", ".join(write_perms) if write_perms else "read-only") + ". "
+ "Blast radius is reduced — no access to publishing secrets or contents:write.")
+ else:
+ return ("HIGH",
+ "Self-hosted runner with PR trigger. External contributors can "
+ "execute arbitrary code on self-hosted infrastructure with broad permissions.")
def check_permissions(content):
diff --git a/repos/apache/github-review/agents/tests/test_security_checks.py b/repos/apache/github-review/agents/tests/test_security_checks.py
index f9eb3ba..d947d33 100644
--- a/repos/apache/github-review/agents/tests/test_security_checks.py
+++ b/repos/apache/github-review/agents/tests/test_security_checks.py
@@ -95,11 +95,29 @@ TESTS = [
# ===== self_hosted =====
{
- "name": "self-hosted HIGH: PR trigger",
+ "name": "self-hosted HIGH: broad perms + auto trigger",
"fixture": ("synthetic", "self-hosted-high-pr.yml"),
"check": "self_hosted",
"expected_severity": "HIGH",
},
+ {
+ "name": "self-hosted MEDIUM: limited perms + auto trigger",
+ "fixture": ("synthetic", "self-hosted-medium-limited-perms.yml"),
+ "check": "self_hosted",
+ "expected_severity": "MEDIUM",
+ },
+ {
+ "name": "self-hosted MEDIUM: broad perms + maintainer-gated (labeled)",
+ "fixture": ("synthetic", "self-hosted-medium-labeled.yml"),
+ "check": "self_hosted",
+ "expected_severity": "MEDIUM",
+ },
+ {
+ "name": "self-hosted LOW: limited perms + maintainer-gated",
+ "fixture": ("synthetic", "self-hosted-low-both-mitigations.yml"),
+ "check": "self_hosted",
+ "expected_severity": "LOW",
+ },
{
"name": "self-hosted INFO: push-only trigger",
"fixture": ("synthetic", "self-hosted-info-push.yml"),
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@tooling.apache.org
For additional commands, e-mail: commits-help@tooling.apache.org