diff --git a/0024-codeowners-app-matrix/.gitignore b/0024-codeowners-app-matrix/.gitignore new file mode 100644 index 0000000..87e7455 --- /dev/null +++ b/0024-codeowners-app-matrix/.gitignore @@ -0,0 +1,4 @@ +*.pem +__pycache__/ +.env +results.json diff --git a/0024-codeowners-app-matrix/README.md b/0024-codeowners-app-matrix/README.md new file mode 100644 index 0000000..7cb7d91 --- /dev/null +++ b/0024-codeowners-app-matrix/README.md @@ -0,0 +1,105 @@ +--- +title: "24. CODEOWNERS + GitHub App approval matrix" +status: Concluded +topics: + - github-apps + - security + - branch-protection +--- + +# 24. CODEOWNERS + GitHub App approval matrix + +Date: 2026-07-13 + +Related: [Auto-merge ADR PR](https://github.com/fullsend-ai/fullsend/pull/2791), +[CODEOWNERS blank-owner PR](https://github.com/fullsend-ai/fullsend/pull/2790) + +## Hypotheses + +| ID | Statement | Result | +|----|-----------|--------| +| H1 | A GitHub App with `contents: read` can submit an APPROVE review, but it does NOT count toward the "require approvals" branch protection rule. | Confirmed | +| H2 | A GitHub App with `contents: write` can submit an APPROVE review, and it DOES count toward "require approvals". | Confirmed | +| H3 | A blank-owner CODEOWNERS entry removes the code-owner review requirement for PRs that only touch that file. | Confirmed | +| H4 | A PR touching both a blank-owner file and an owned file still requires code-owner approval for the owned file. | Confirmed | +| H5 | When H2 and H3 are both true, a `contents: write` app is the sole approver on a blank-owner-only PR, and the PR is mergeable. | Confirmed | +| H6 | When a PR touches both blank-owner and owned files, even a `contents: write` app cannot satisfy the code-owner requirement for the owned files. | Confirmed | + +## Approach + +### Test infrastructure + +- **Org:** `appdumpster` +- **Repo:** `appdumpster/codeowners-lab` +- **Team:** `@appdumpster/owners` (with the human operator as a member) +- **Apps:** + - `appdumpster-read-bot` — `contents: read`, `pull_requests: write` + - `appdumpster-write-bot` — `contents: write`, `pull_requests: write` + +### Repo layout + +``` +CODEOWNERS: + * @appdumpster/owners + go.mod + go.sum + +Files: main.go, go.mod, go.sum +``` + +### Branch protection on `main` + +- Require 1 approval +- Require review from code owners +- No status checks (keep it simple) + +### Test matrix + +| PR | Branch | Files touched | Approved by | Tests | +|----|--------|--------------|-------------|-------| +| 1 | `test/read-owned` | `main.go` | read-bot | H1 | +| 2 | `test/write-owned` | `main.go` | write-bot | H2, H4 partial | +| 3 | `test/read-blank` | `go.mod` | read-bot | H1, H3 | +| 4 | `test/write-blank` | `go.mod` | write-bot | H2, H3, H5 | +| 5 | `test/read-mixed` | `go.mod` + `main.go` | read-bot | H1, H4 | +| 6 | `test/write-mixed` | `go.mod` + `main.go` | write-bot | H2, H4, H6 | + +### Measurement + +For each PR, after the bot approves, query: +- `GET /repos/{owner}/{repo}/pulls/{number}/reviews` — confirm the review exists +- `GET /repos/{owner}/{repo}/pulls/{number}/merge` — can it merge? (405 = no, 204 = yes) +- `PUT /repos/{owner}/{repo}/pulls/{number}/merge` — attempt the merge, record status code and error message + +## Results + +Ran on 2026-07-13 against `appdumpster/codeowners-lab` with two throwaway GitHub Apps. + +| PR | Test | `mergeable_state` | Merge attempt | HTTP | Error message | +|----|------|--------------------|---------------|------|---------------| +| #1 | read-bot + owned file | `blocked` | BLOCKED | 403 | Resource not accessible by integration | +| #2 | write-bot + owned file | `blocked` | BLOCKED | 405 | Waiting on code owner review from appdumpster/owners. | +| #3 | read-bot + blank-owner file | `blocked` | BLOCKED | 403 | Resource not accessible by integration | +| #4 | write-bot + blank-owner file | `clean` | **MERGED** | 200 | Pull Request successfully merged | +| #5 | read-bot + mixed files | `unknown` | BLOCKED | 403 | Resource not accessible by integration | +| #6 | write-bot + mixed files | `unknown` | BLOCKED | 405 | Waiting on code owner review from appdumpster/owners. | + +### Key observations + +1. **`contents: read` vs `contents: write` produces different failure modes.** The read-bot gets 403 "Resource not accessible by integration" — a permission error, not a branch protection error. The write-bot gets 405 with a specific branch protection message. This means the read-bot's approval is invisible to branch protection; it can't even attempt the merge. + +2. **Blank-owner CODEOWNERS entries work as documented.** PR #4 (write-bot approving a PR that only touches `go.mod`) had `mergeable_state: clean` and merged successfully. The blank-owner entry removed the code-owner review gate. + +3. **Blank-owner doesn't leak to owned files.** PRs #5 and #6 (mixed: `go.mod` + `main.go`) were both blocked. The write-bot's approval satisfied the general approval requirement but not the code-owner requirement for `main.go`. + +4. **The only mergeable combination is `contents: write` app + blank-owner-only files.** This is exactly the scenario the [auto-merge ADR](https://github.com/fullsend-ai/fullsend/pull/2791) targets. + +## Conclusion + +All six hypotheses confirmed. the [auto-merge ADR](https://github.com/fullsend-ai/fullsend/pull/2791)'s design is sound: + +- A `contents: write` merge app can approve and merge PRs that only touch blank-owner CODEOWNERS files (like `go.mod`/`go.sum`). +- The same app cannot bypass code-owner review for files with actual owners. +- A `contents: read` review app's approvals are ignored by branch protection entirely. + +The combination of blank-owner CODEOWNERS entries + a write-capable merge app is the minimum viable path to bot-driven auto-merge for dependency updates without compromising code-owner review for application code. diff --git a/0024-codeowners-app-matrix/create_prs.sh b/0024-codeowners-app-matrix/create_prs.sh new file mode 100755 index 0000000..4d7bcc0 --- /dev/null +++ b/0024-codeowners-app-matrix/create_prs.sh @@ -0,0 +1,100 @@ +#!/usr/bin/env bash +set -euo pipefail + +OWNER="appdumpster" +REPO="codeowners-lab" +BASE="main" + +MAIN_GO_CONTENT=$(printf 'package main\n\nfunc main() {\n\t// modified\n}\n' | base64 -w0) +GO_MOD_CONTENT=$(printf 'module github.com/appdumpster/codeowners-lab\n\ngo 1.23\n' | base64 -w0) + +PR_BODY="Automated test PR for CODEOWNERS experiment." + +echo "=== Fetching SHA of main branch" +MAIN_SHA=$(gh api "repos/${OWNER}/${REPO}/git/ref/heads/${BASE}" --jq '.object.sha') +echo "=== main SHA: ${MAIN_SHA}" + +create_pr() { + local branch="$1" + local title="$2" + local file1="$3" + local content1="$4" + local file2="${5:-}" + local content2="${6:-}" + + echo "=== Creating PR: ${title}" + + # Check if branch already exists + if gh api "repos/${OWNER}/${REPO}/git/ref/heads/${branch}" > /dev/null 2>&1; then + echo "=== Branch ${branch} already exists, skipping" + return + fi + + # Create branch ref + echo "=== Creating branch ${branch}" + gh api --method POST "repos/${OWNER}/${REPO}/git/refs" \ + --field "ref=refs/heads/${branch}" \ + --field "sha=${MAIN_SHA}" > /dev/null + + # Get current file SHA and commit change for file1 + echo "=== Modifying ${file1}" + local file1_sha + file1_sha=$(gh api "repos/${OWNER}/${REPO}/contents/${file1}?ref=${branch}" --jq '.sha') + gh api --method PUT "repos/${OWNER}/${REPO}/contents/${file1}" \ + --field "message=test: modify ${file1}" \ + --field "content=${content1}" \ + --field "sha=${file1_sha}" \ + --field "branch=${branch}" > /dev/null + + # If a second file is specified, do the same + if [[ -n "${file2}" ]]; then + echo "=== Modifying ${file2}" + local file2_sha + file2_sha=$(gh api "repos/${OWNER}/${REPO}/contents/${file2}?ref=${branch}" --jq '.sha') + gh api --method PUT "repos/${OWNER}/${REPO}/contents/${file2}" \ + --field "message=test: modify ${file2}" \ + --field "content=${content2}" \ + --field "sha=${file2_sha}" \ + --field "branch=${branch}" > /dev/null + fi + + # Create PR + echo "=== Opening PR" + gh api --method POST "repos/${OWNER}/${REPO}/pulls" \ + --field "title=${title}" \ + --field "body=${PR_BODY}" \ + --field "head=${branch}" \ + --field "base=${BASE}" > /dev/null + + echo "=== Done: ${title}" +} + +create_pr "test/read-owned" \ + "H1: read-bot approves owned file (main.go)" \ + "main.go" "${MAIN_GO_CONTENT}" + +create_pr "test/write-owned" \ + "H2: write-bot approves owned file (main.go)" \ + "main.go" "${MAIN_GO_CONTENT}" + +create_pr "test/read-blank" \ + "H1+H3: read-bot approves blank-owner file (go.mod)" \ + "go.mod" "${GO_MOD_CONTENT}" + +create_pr "test/write-blank" \ + "H2+H3+H5: write-bot approves blank-owner file (go.mod)" \ + "go.mod" "${GO_MOD_CONTENT}" + +create_pr "test/read-mixed" \ + "H1+H4: read-bot approves mixed files (go.mod + main.go)" \ + "go.mod" "${GO_MOD_CONTENT}" \ + "main.go" "${MAIN_GO_CONTENT}" + +create_pr "test/write-mixed" \ + "H2+H4+H6: write-bot approves mixed files (go.mod + main.go)" \ + "go.mod" "${GO_MOD_CONTENT}" \ + "main.go" "${MAIN_GO_CONTENT}" + +echo "" +echo "=== Open PRs:" +gh api "repos/${OWNER}/${REPO}/pulls?state=open" --jq '.[] | "#\(.number) \(.title)"' diff --git a/0024-codeowners-app-matrix/requirements.txt b/0024-codeowners-app-matrix/requirements.txt new file mode 100644 index 0000000..e02b4e2 --- /dev/null +++ b/0024-codeowners-app-matrix/requirements.txt @@ -0,0 +1,3 @@ +PyJWT>=2.8,<3 +cryptography>=42,<45 +requests>=2.31,<3 diff --git a/0024-codeowners-app-matrix/run_matrix.py b/0024-codeowners-app-matrix/run_matrix.py new file mode 100755 index 0000000..8ea92dc --- /dev/null +++ b/0024-codeowners-app-matrix/run_matrix.py @@ -0,0 +1,267 @@ +#!/usr/bin/env python3 +"""Test harness for CODEOWNERS + GitHub App permission matrix. + +Authenticates as two GitHub Apps (read-bot and write-bot), submits APPROVE +reviews on test PRs, and checks whether each PR becomes mergeable. +""" + +import json +import os +import sys +import time +from pathlib import Path + +import jwt +import requests + +ORG = "appdumpster" +REPO = "codeowners-lab" +API = "https://api.github.com" +HEADERS = { + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", +} + +TESTS = [ + { + "branch": "test/read-owned", + "bot": "read", + "description": "H1: read-bot approves owned file", + "hypotheses": ["H1"], + }, + { + "branch": "test/write-owned", + "bot": "write", + "description": "H2: write-bot approves owned file", + "hypotheses": ["H2"], + }, + { + "branch": "test/read-blank", + "bot": "read", + "description": "H1+H3: read-bot approves blank-owner file", + "hypotheses": ["H1", "H3"], + }, + { + "branch": "test/write-blank", + "bot": "write", + "description": "H2+H3+H5: write-bot approves blank-owner file", + "hypotheses": ["H2", "H3", "H5"], + }, + { + "branch": "test/read-mixed", + "bot": "read", + "description": "H1+H4: read-bot approves mixed files", + "hypotheses": ["H1", "H4"], + }, + { + "branch": "test/write-mixed", + "bot": "write", + "description": "H2+H4+H6: write-bot approves mixed files", + "hypotheses": ["H2", "H4", "H6"], + }, +] + + +def make_jwt(app_id: str, pem_path: str) -> str: + """Create a JWT for a GitHub App using PyJWT.""" + with open(pem_path, "rb") as f: + private_key = f.read() + now = int(time.time()) + payload = { + "iat": now - 60, + "exp": now + 600, + "iss": app_id, + } + return jwt.encode(payload, private_key, algorithm="RS256") + + +def get_installation_token(app_jwt: str) -> str: + """Get an installation access token for the target org.""" + resp = requests.get( + f"{API}/app/installations", + headers={**HEADERS, "Authorization": f"Bearer {app_jwt}"}, + timeout=60, + ) + resp.raise_for_status() + + for inst in resp.json(): + if inst.get("account", {}).get("login") == ORG: + token_resp = requests.post( + f"{API}/app/installations/{inst['id']}/access_tokens", + headers={**HEADERS, "Authorization": f"Bearer {app_jwt}"}, + timeout=60, + ) + token_resp.raise_for_status() + return token_resp.json()["token"] + + raise RuntimeError(f"No installation found for org '{ORG}'") + + +def approve_pr(token: str, pr_number: int, bot_name: str) -> dict: + """Submit an APPROVE review on a PR.""" + resp = requests.post( + f"{API}/repos/{ORG}/{REPO}/pulls/{pr_number}/reviews", + headers={**HEADERS, "Authorization": f"Bearer {token}"}, + json={ + "event": "APPROVE", + "body": f"Approved by {bot_name} (experiment)", + }, + timeout=60, + ) + return {"status": resp.status_code, "body": resp.json()} + + +def check_merge_status(token: str, pr_number: int) -> dict: + """Check PR mergeability and attempt a squash merge.""" + pr_resp = requests.get( + f"{API}/repos/{ORG}/{REPO}/pulls/{pr_number}", + headers={**HEADERS, "Authorization": f"Bearer {token}"}, + timeout=60, + ) + pr_data = pr_resp.json() + + reviews_resp = requests.get( + f"{API}/repos/{ORG}/{REPO}/pulls/{pr_number}/reviews", + headers={**HEADERS, "Authorization": f"Bearer {token}"}, + timeout=60, + ) + reviews = [ + {"user": r["user"]["login"], "state": r["state"]} for r in reviews_resp.json() + ] + + merge_resp = requests.put( + f"{API}/repos/{ORG}/{REPO}/pulls/{pr_number}/merge", + headers={**HEADERS, "Authorization": f"Bearer {token}"}, + json={"merge_method": "squash"}, + timeout=60, + ) + + return { + "mergeable": pr_data.get("mergeable"), + "mergeable_state": pr_data.get("mergeable_state"), + "reviews": reviews, + "merge_status": merge_resp.status_code, + "merge_body": merge_resp.json(), + } + + +def main(): + read_app_id = os.environ.get("READ_BOT_APP_ID") + read_pem = os.environ.get("READ_BOT_PEM") + write_app_id = os.environ.get("WRITE_BOT_APP_ID") + write_pem = os.environ.get("WRITE_BOT_PEM") + + missing = [ + name + for name, val in [ + ("READ_BOT_APP_ID", read_app_id), + ("READ_BOT_PEM", read_pem), + ("WRITE_BOT_APP_ID", write_app_id), + ("WRITE_BOT_PEM", write_pem), + ] + if not val + ] + if missing: + print( + f"ERROR: required env vars not set: {', '.join(missing)}", file=sys.stderr + ) + sys.exit(1) + + # Authenticate both apps + print("Authenticating read-bot...") + read_jwt = make_jwt(read_app_id, read_pem) + read_token = get_installation_token(read_jwt) + print(" read-bot authenticated") + + print("Authenticating write-bot...") + write_jwt = make_jwt(write_app_id, write_pem) + write_token = get_installation_token(write_jwt) + print(" write-bot authenticated") + + tokens = {"read": read_token, "write": write_token} + + # List open PRs to build branch -> PR number mapping + list_token = ( + os.environ.get("GH_TOKEN") or os.environ.get("GITHUB_TOKEN") or write_token + ) + resp = requests.get( + f"{API}/repos/{ORG}/{REPO}/pulls", + headers={**HEADERS, "Authorization": f"Bearer {list_token}"}, + params={"state": "open", "per_page": 100}, + timeout=60, + ) + resp.raise_for_status() + branch_to_pr = {pr["head"]["ref"]: pr["number"] for pr in resp.json()} + print(f"\nOpen PRs: {json.dumps(branch_to_pr, indent=2)}\n") + + # Run test matrix + results = [] + for test in TESTS: + branch = test["branch"] + pr_number = branch_to_pr.get(branch) + if not pr_number: + print(f"SKIP: no open PR found for branch '{branch}'") + results.append({**test, "pr": None, "skipped": True}) + continue + + print("=" * 60) + print(f"TEST: {test['description']}") + print(f" Branch: {branch} PR: #{pr_number} Bot: {test['bot']}") + print("=" * 60) + + token = tokens[test["bot"]] + bot_name = f"{test['bot']}-bot" + + # Approve + approval = approve_pr(token, pr_number, bot_name) + print(f" Approval: {approval['status']}") + if approval["status"] != 200: + print(f" Approval body: {json.dumps(approval['body'], indent=2)}") + + # Wait for GitHub to process + time.sleep(3) + + # Check merge status + status = check_merge_status(token, pr_number) + print(f" Mergeable: {status['mergeable']}") + print(f" Mergeable state: {status['mergeable_state']}") + print(f" Reviews: {json.dumps(status['reviews'], indent=2)}") + print(f" Merge attempt: {status['merge_status']}") + print(f" Merge body: {json.dumps(status['merge_body'], indent=2)}") + + results.append( + { + **test, + "pr": pr_number, + "skipped": False, + "approval": approval, + "merge_status": status["merge_status"], + "mergeable": status["mergeable"], + "mergeable_state": status["mergeable_state"], + "reviews": status["reviews"], + "merge_body": status["merge_body"], + } + ) + + # Summary + print("\n" + "=" * 60) + print("SUMMARY") + print("=" * 60) + for r in results: + if r.get("skipped"): + print(f" SKIP {r['description']}") + continue + merged = r["merge_status"] == 200 + label = "MERGED" if merged else "BLOCKED" + msg = r["merge_body"].get("message", "") + print(f" {label} PR#{r['pr']} {r['description']} -- {msg}") + + # Write results + results_path = Path(__file__).resolve().parent / "results.json" + with open(results_path, "w") as f: + json.dump(results, f, indent=2) + print(f"\nResults written to {results_path}") + + +if __name__ == "__main__": + main() diff --git a/0024-codeowners-app-matrix/setup_repo.sh b/0024-codeowners-app-matrix/setup_repo.sh new file mode 100755 index 0000000..0d4571b --- /dev/null +++ b/0024-codeowners-app-matrix/setup_repo.sh @@ -0,0 +1,106 @@ +#!/usr/bin/env bash +set -euo pipefail + +ORG="appdumpster" +REPO="codeowners-lab" +TEAM="owners" +FULL_REPO="${ORG}/${REPO}" + +# --- 1. Create team --- +echo "=== Creating team @${ORG}/${TEAM} (if not exists)" +if gh api "orgs/${ORG}/teams/${TEAM}" --silent 2>/dev/null; then + echo " Team already exists, skipping." +else + gh api "orgs/${ORG}/teams" \ + -f name="${TEAM}" \ + -f privacy="closed" \ + --silent + echo " Team created." +fi + +# --- 2. Create repo --- +echo "=== Creating repo ${FULL_REPO} (if not exists)" +if gh repo view "${FULL_REPO}" --json name --jq .name &>/dev/null; then + echo " Repo already exists, skipping." +else + gh repo create "${FULL_REPO}" --public + echo " Repo created." +fi + +# --- 3. Grant team write access --- +echo "=== Granting team @${ORG}/${TEAM} write access to ${FULL_REPO}" +gh api "orgs/${ORG}/teams/${TEAM}/repos/${FULL_REPO}" \ + -X PUT \ + -f permission="push" \ + --silent +echo " Done." + +# --- 4. Seed files on main branch --- +echo "=== Seeding files on main branch (if main doesn't exist yet)" +if gh api "repos/${FULL_REPO}/branches/main" --silent 2>/dev/null; then + echo " Main branch already exists, skipping seed." +else + seed_file() { + local path="$1" + local content="$2" + local encoded + encoded=$(printf '%s' "${content}" | base64 -w0) + gh api "repos/${FULL_REPO}/contents/${path}" \ + -X PUT \ + -f message="seed: add ${path}" \ + -f content="${encoded}" \ + --silent + echo " Seeded ${path}" + } + + seed_file "main.go" "package main + +func main() {} +" + + seed_file "go.mod" "module github.com/appdumpster/codeowners-lab + +go 1.22 +" + + seed_file "go.sum" "// empty +" + + seed_file "CODEOWNERS" "* @appdumpster/owners + +go.mod +go.sum +" +fi + +# --- 5. Configure branch protection on main --- +echo "=== Configuring branch protection on main" +gh api "repos/${FULL_REPO}/branches/main/protection" \ + -X PUT \ + --input - <<'JSON' +{ + "required_pull_request_reviews": { + "required_approving_review_count": 1, + "require_code_owner_reviews": true, + "dismiss_stale_reviews": false, + "require_last_push_approval": false + }, + "enforce_admins": false, + "required_status_checks": null, + "restrictions": null +} +JSON +echo " Branch protection configured." + +# --- Summary --- +echo "" +echo "=== Setup complete" +echo " Repo URL: https://github.com/${FULL_REPO}" +echo " Branch protection on main:" +echo " - Required approving reviews: 1" +echo " - Require code owner reviews: true" +echo " - Dismiss stale reviews: false" +echo " - Require last push approval: false" +echo " - Enforce admins: false" +echo " - Required status checks: null" +echo " - Restrictions: null" diff --git a/0024-codeowners-app-matrix/teardown.sh b/0024-codeowners-app-matrix/teardown.sh new file mode 100755 index 0000000..87b7c28 --- /dev/null +++ b/0024-codeowners-app-matrix/teardown.sh @@ -0,0 +1,15 @@ +#!/usr/bin/env bash +set -euo pipefail + +ORG="appdumpster" +REPO="codeowners-lab" +TEAM="owners" + +echo "=== Deleting repo ${ORG}/${REPO} ===" +gh repo delete "${ORG}/${REPO}" --yes 2>/dev/null || echo " (not found)" + +echo "=== Deleting team @${ORG}/${TEAM} ===" +gh api "orgs/${ORG}/teams/${TEAM}" --method DELETE 2>/dev/null || echo " (not found)" + +echo "=== Done ===" +echo "Note: GitHub Apps must be deleted manually from https://github.com/organizations/${ORG}/settings/apps" diff --git a/README.md b/README.md index 2323d9a..7e02750 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,7 @@ Experiments for the fullsend platform — each tests a hypothesis about autonomo | 0021 | [Tool scoping](0021-tool-scoping/) | Concluded | | 0022 | [Claude GitHub App auth](0022-claude-github-app-auth/) | Concluded | | 0023 | [Review cache publication policy](0023-review-cache/) | Concluded | +| 0024 | [CODEOWNERS + GitHub App approval matrix](0024-codeowners-app-matrix/) | Concluded | ## Conventions