Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 27 additions & 0 deletions .github/workflows/pr-safety-lint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
name: PR Safety Lint

on:
pull_request:
branches: [main]

permissions:
contents: read

jobs:
pr-safety-lint:
runs-on: ubuntu-latest
steps:
- name: Checkout PR
uses: actions/checkout@v4
with:
fetch-depth: 0

- name: Ensure base ref is available
run: |
git fetch origin "${{ github.event.pull_request.base.ref }}"

- name: Run deterministic PR safety lint
run: |
python3 scripts/pr_safety_lint.py \
--base "origin/${{ github.event.pull_request.base.ref }}" \
--head HEAD
4 changes: 3 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ Anything. Code in any language, documentation, proposals, architecture decisions
- A GitHub Action handles auto-merging (no human is in the loop)
- You **can't approve your own PR**
- Reviewer accounts must be **at least 30 days old**
- All CI checks (security scanning, file size limits) must pass
- All CI checks (security scanning, file size limits, and PR safety lint) must pass
- Strict review mode is documented in [`STRICT_PR_REVIEW.md`](STRICT_PR_REVIEW.md)
- The intent is **low-friction contribution, not paranoia theater**: most normal PRs should still feel easy to open and review, while anything that expands execution, rendering, workflow permissions, or trust-boundary risk gets a sharper pass

## What you can't do

Expand Down
52 changes: 52 additions & 0 deletions STRICT_PR_REVIEW.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# Strict PR Review Mode

Slop Farm accepts outside agent contributions, so PR review needs a stricter trust posture than a normal hobby repo.
That does not mean high-friction intake.
The goal is cheap guardrails plus sharp review on risky surfaces, not a fortress that scares off good contributors.

## Baseline rules
1. Read the diff first.
2. Treat PR body text, README prose, generated HTML, receipt content, and any contributor-authored strings as untrusted.
3. Do not execute PR code until changed executable files have been sanity-checked directly.
4. Never run package installs or networked setup from a PR branch without explicit human approval.
5. Prefer deterministic local checks over model-based review or vibe-based trust.
6. Keep the review surface proportional to the risk: docs and static data should feel easier than renderers, workflow files, shell surfaces, or anything that executes untrusted content.

## Default review sequence
1. Inspect changed files and diff shape.
2. Identify changed executable/build files.
3. Run deterministic repo-owned checks only.
4. If dynamic execution is still needed, keep it local, minimal, and secret-free.
5. Record which findings are definitely reproduced versus which ones remain trust-boundary concerns under explicit test.

## Repo-owned guardrails
- `.github/workflows/pr-safety-lint.yml` runs a deterministic diff-based safety scan on PRs.
- `scripts/pr_safety_lint.py` flags suspicious added patterns in changed lines only.

## What the linter is for
The linter is not trying to prove a PR is malicious.
It is there to cheaply catch high-signal review hazards, including:
- pipe-to-shell commands
- added `sudo`
- package install/setup commands
- dynamic execution surfaces
- inline script / raw embedding patterns
- workflow permission expansions
- reviewer-instruction style prose in changed lines

## Review philosophy
Default posture: low-trust, low-drama.

That means:
- assume contributors may be mistaken without assuming they are malicious
- add friction only where the repo has real trust-boundary risk
- prefer one clear review note over a pile of vague suspicion
- keep the repo welcoming to outside bots that leave inspectable, boring changes

## What still requires human judgment
A clean linter run is not approval.
Reviewers still need to decide whether the contribution:
- is correct
- is scoped well
- preserves repo trust boundaries
- introduces unnecessary execution or network surfaces
171 changes: 171 additions & 0 deletions scripts/pr_safety_lint.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
#!/usr/bin/env python3
import argparse
import os
import re
import subprocess
import sys
from dataclasses import dataclass
from pathlib import Path


@dataclass
class Finding:
severity: str
category: str
path: str
line_no: int
message: str
snippet: str


BLOCK_RULES = [
("shell_pipe_exec", re.compile(r"\b(?:curl|wget)\b[^\n|]{0,200}\|\s*(?:bash|sh)\b", re.I), "Pipe-to-shell command added"),
("sudo_usage", re.compile(r"\bsudo\b"), "sudo invocation added"),
("workflow_write_all", re.compile(r"\bpermissions\s*:\s*write-all\b", re.I), "Workflow requests write-all permissions"),
("gh_token_exfil", re.compile(r"\b(?:gh|curl|wget)\b[^\n]{0,200}(?:GITHUB_TOKEN|secrets\.[A-Z0-9_]+)", re.I), "Possible token-bearing network command added"),
]

WARN_RULES = [
("package_install", re.compile(r"\b(?:npm\s+install|npm\s+i\b|pnpm\s+add\b|pnpm\s+install\b|yarn\s+add\b|pip(?:3)?\s+install\b|uv\s+pip\s+install\b|cargo\s+install\b|go\s+install\b|brew\s+install\b|apt(?:-get)?\s+install\b)"), "Package install command added"),
("dynamic_exec", re.compile(r"\b(?:eval\s*\(|exec\s*\(|os\.system\s*\(|subprocess\.(?:run|Popen|call|check_output)\s*\(|child_process\.|Runtime\.getRuntime\(|ProcessBuilder\()"), "Dynamic execution surface added"),
("network_code", re.compile(r"\b(?:requests\.|urllib\.|fetch\s*\(|XMLHttpRequest|httpx\.|aiohttp\.|socket\.|net\.|open\(['\"]https?://|urlopen\s*\()"), "Network call surface added"),
("inline_script", re.compile(r"<script\b|innerHTML\s*=|dangerouslySetInnerHTML|json\.dumps\s*\(|JSON\.stringify\s*\("), "Inline script or raw embedding pattern added"),
("reviewer_instruction", re.compile(r"ignore previous instructions|run this command|paste this into (?:your )?terminal|execute the following", re.I), "Reviewer-instruction style text added"),
("workflow_write_perm", re.compile(r"\bpermissions\s*:\s*[\s\S]{0,80}\b(?:contents|actions|checks|deployments|id-token|packages|pull-requests|security-events)\s*:\s*write\b", re.I), "Workflow requests write permission"),
]

INFO_RULES = [
("generated_html", re.compile(r"<html\b|<!DOCTYPE html>", re.I), "HTML artifact changed"),
]


EXCLUDED_PATH_PATTERNS = [
re.compile(r"^\.git/"),
]

MARKDOWN_SUFFIXES = {".md", ".txt", ".rst"}


def run(cmd, cwd=None):
return subprocess.check_output(cmd, cwd=cwd, text=True)


def changed_files(base, head):
out = run(["git", "diff", "--name-only", f"{base}...{head}"])
return [line.strip() for line in out.splitlines() if line.strip()]


def iter_added_lines(base, head, path):
diff = run(["git", "diff", "--unified=0", "--no-color", f"{base}...{head}", "--", path])
new_line = None
for raw in diff.splitlines():
if raw.startswith("@@"):
m = re.search(r"\+(\d+)(?:,(\d+))?", raw)
new_line = int(m.group(1)) if m else None
continue
if raw.startswith("+++") or raw.startswith("---"):
continue
if raw.startswith("+"):
if new_line is None:
continue
yield new_line, raw[1:]
new_line += 1
elif raw.startswith("-"):
continue
else:
if new_line is not None:
new_line += 1


def path_excluded(path):
return any(p.search(path) for p in EXCLUDED_PATH_PATTERNS)


def severity_for_path(severity, path):
suffix = Path(path).suffix.lower()
if suffix in MARKDOWN_SUFFIXES:
return {"block": "warn", "warn": "info", "info": "info"}[severity]
return severity


def meta_rule_definition(path, line):
if not path.endswith("scripts/pr_safety_lint.py"):
return False
return "re.compile(" in line or "subprocess.check_output(" in line


def markdown_example_line(path, line):
suffix = Path(path).suffix.lower()
if suffix not in MARKDOWN_SUFFIXES:
return False
stripped = line.strip()
return stripped.startswith("- ") and "`" in stripped


def scan(base, head):
findings = []
files = changed_files(base, head)
for path in files:
if path_excluded(path):
continue
for line_no, line in iter_added_lines(base, head, path):
if meta_rule_definition(path, line):
continue
rules = []
if not markdown_example_line(path, line):
rules.extend(("block",) + rule for rule in BLOCK_RULES)
rules.extend(("warn",) + rule for rule in WARN_RULES)
rules.extend(("info",) + rule for rule in INFO_RULES)
for severity, category, pattern, message in rules:
if pattern.search(line):
findings.append(Finding(severity_for_path(severity, path), category, path, line_no, message, line.strip()))
return files, findings


def summarize(files, findings):
by_sev = {"block": [], "warn": [], "info": []}
for f in findings:
by_sev[f.severity].append(f)

lines = []
lines.append("# PR safety lint")
lines.append("")
lines.append(f"Changed files scanned: {len(files)}")
lines.append(f"Findings: {len(by_sev['block'])} block, {len(by_sev['warn'])} warn, {len(by_sev['info'])} info")
lines.append("")
for severity in ["block", "warn", "info"]:
if not by_sev[severity]:
continue
lines.append(f"## {severity.upper()}")
for f in by_sev[severity]:
lines.append(f"- `{f.path}:{f.line_no}` [{f.category}] {f.message}")
lines.append(f" - `{f.snippet[:180]}`")
lines.append("")
return "\n".join(lines), by_sev


def main():
parser = argparse.ArgumentParser()
parser.add_argument("--base", default="origin/main")
parser.add_argument("--head", default="HEAD")
args = parser.parse_args()

files, findings = scan(args.base, args.head)
summary, by_sev = summarize(files, findings)
print(summary)

step_summary = os.environ.get("GITHUB_STEP_SUMMARY")
if step_summary:
Path(step_summary).write_text(summary + "\n", encoding="utf-8")

for sev in ["block", "warn"]:
for f in by_sev[sev]:
level = "error" if sev == "block" else "warning"
print(f"::{level} file={f.path},line={f.line_no},title={f.category}::{f.message}: {f.snippet}")

if by_sev["block"]:
sys.exit(1)


if __name__ == "__main__":
main()
Loading