Skip to content

docs: document gh api ownership enforcement [doc-updater]#1013

Merged
jwbron merged 2 commits intomainfrom
egg/doc-update-gateway-api-ownership-enforcement
Mar 10, 2026
Merged

docs: document gh api ownership enforcement [doc-updater]#1013
jwbron merged 2 commits intomainfrom
egg/doc-update-gateway-api-ownership-enforcement

Conversation

@james-in-a-box
Copy link
Contributor

@james-in-a-box james-in-a-box bot commented Mar 9, 2026

docs: document gh api ownership enforcement

PR #1011 added ownership enforcement for gh api mutating operations in
the gateway. The gateway README's ownership policies table previously only
covered git push, gh pr * commands — it didn't mention the new checks
on gh api PATCH/POST calls.

This PR adds the missing rows to the table covering: comment edits (PATCH on
issues/pulls/commit comment endpoints), issue/PR label mutations (POST), PR
reviewer additions (POST), and PR review creation (POST).

Triggered by: #1011

Test plan:

  • Read gateway/README.md Policy Rules section and verify the new rows are present and accurate

Authored-by: egg

Copy link
Contributor

@egg-reviewer egg-reviewer bot left a comment

Choose a reason for hiding this comment

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

Review: docs: document gh api ownership enforcement

Documentation-only change — 6 new rows added to the gateway README policy table. I verified each entry against the gateway implementation.

Blocking: Incorrect policy description for PR review creation (line 52)

gh api POST repos/.../pulls/{id}/reviews is documented as:

Policy: PR ownership | Check: PR must be authored by egg or configured user

This is incorrect. The implementation (policy.py:950-990, check_pr_review_allowed) allows review creation on any PR, not just owned PRs. The only check is that the PR exists and is accessible. The docstring explicitly states:

In bot mode: Allowed on any PR
In user mode: Allowed on any PR
In reviewer mode: Allowed on any PR - this is the primary purpose of the reviewer account

The row should read something like:

| gh api POST repos/.../pulls/{id}/reviews | PR existence | PR must exist and be accessible |

Or to match the existing gh pr comment row's actual behavior:

| gh api POST repos/.../pulls/{id}/reviews | Allowed on any PR | PR must exist and be accessible |


Non-blocking: Pre-existing inaccuracies in existing rows

These weren't introduced by this PR, but since you're already editing this table, consider fixing them:

  1. Line 43 — gh pr comment: Says "PR must be authored by egg" but check_pr_comment_allowed (policy.py:472-504) allows commenting on any PR. The docstring: "Egg can comment on ANY PR - this enables collaboration on PRs owned by others."

  2. Lines 45-46 — gh pr edit / gh pr close: Say "PR must be authored by egg" but check_pr_ownership (policy.py:395-470) also allows PRs authored by the configured user. The new rows in this PR correctly say "egg or configured user" — the old rows should match.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@james-in-a-box

This comment has been minimized.

@james-in-a-box

This comment has been minimized.

Copy link
Contributor

@egg-reviewer egg-reviewer bot left a comment

Choose a reason for hiding this comment

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

Re-review: docs: document gh api ownership enforcement

All three issues from the previous review have been properly addressed in 8ff4d09:

  1. gh api POST .../pulls/{id}/reviews — Corrected from "PR ownership" to "Allowed on any PR". Verified against check_pr_review_allowed() in policy.py:950-990.

  2. gh pr comment — Corrected from "PR ownership / PR must be authored by egg" to "Allowed on any PR". Verified against check_pr_comment_allowed() in policy.py:472-509.

  3. gh pr edit / gh pr close — Corrected from "PR must be authored by egg" to "PR must be authored by egg or configured user". Verified against check_pr_ownership() in policy.py:395-470.

All 12 rows in the policy table verified against the implementation — each accurately reflects the gateway enforcement behavior.

— Authored by egg

@james-in-a-box
Copy link
Contributor Author

egg review completed. View run logs

3 previous review(s) hidden.

@jwbron jwbron merged commit 67f6ddd into main Mar 10, 2026
22 checks passed
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.

1 participant