From a491fdbc8a69f87ab4632896ff7fa12bc57e8297 Mon Sep 17 00:00:00 2001 From: Jared Atkinson Date: Tue, 18 Aug 2026 22:04:25 -0700 Subject: [PATCH 1/3] BED-9371: restrict reviewer deployment edges to valid code paths Only emit reviewer-originated GH_CanDeployToEnvironment edges when the reviewer can both satisfy the approval gate and independently supply deployable code under the environment branch policy. The lookup now evaluates effective repository access across direct repo assignments, team inheritance, built-in org roles, and custom org-role assignments, then reuses the existing branch write semantics for unrestricted, protected, and custom-policy environments. Add focused coverage for reviewer path resolution and update the edge documentation and schema text to distinguish approval authority from traversable deployment capability. --- descriptions/edges/GH_ApprovesDeploymentTo.md | 2 +- .../edges/GH_CanDeployToEnvironment.md | 6 +- extension/schema.json | 2 +- src/openhound_github/lookup.py | 260 ++++++++++++++++++ src/openhound_github/main.py | 6 + src/openhound_github/models/environment.py | 161 ++++++++++- .../models/environment_branch_policy.py | 13 +- src/openhound_github/transforms.py | 97 ++++++- tests/test_environment_model.py | 98 +++++++ tests/test_environment_reviewer_lookup.py | 172 ++++++++++++ 10 files changed, 797 insertions(+), 20 deletions(-) create mode 100644 tests/test_environment_reviewer_lookup.py diff --git a/descriptions/edges/GH_ApprovesDeploymentTo.md b/descriptions/edges/GH_ApprovesDeploymentTo.md index 52ddb4c..5280374 100644 --- a/descriptions/edges/GH_ApprovesDeploymentTo.md +++ b/descriptions/edges/GH_ApprovesDeploymentTo.md @@ -4,4 +4,4 @@ The non-traversable GH_ApprovesDeploymentTo edge represents that a user or team This edge is emitted from GH_User or GH_Team nodes to GH_Environment nodes when the environment includes a required reviewer protection rule. Required reviewers act as an approval gate before jobs referencing the environment can continue. -The edge is non-traversable because it records reviewer configuration rather than direct deployment access. When self-review is allowed, the same reviewer may also receive a traversable GH_CanDeployToEnvironment edge because they can satisfy the approval gate themselves. When prevent_self_review is enabled, GH_ApprovesDeploymentTo remains context only because the split-principal approval flow is not currently modeled. +The edge is non-traversable because it records reviewer configuration rather than direct deployment access. When self-review is allowed, the same reviewer may also receive a traversable GH_CanDeployToEnvironment edge only if they can also supply deployable code through GH_CanCreateBranch or GH_CanWriteBranch under the environment's branch policy. When prevent_self_review is enabled, GH_ApprovesDeploymentTo remains context only because the split-principal approval flow is not currently modeled. diff --git a/descriptions/edges/GH_CanDeployToEnvironment.md b/descriptions/edges/GH_CanDeployToEnvironment.md index 98e8860..f98b76c 100644 --- a/descriptions/edges/GH_CanDeployToEnvironment.md +++ b/descriptions/edges/GH_CanDeployToEnvironment.md @@ -1,7 +1,9 @@ ## General Information -The traversable GH_CanDeployToEnvironment edge represents the ability for a repository, branch, repository role, or self-approving reviewer to satisfy the modeled deployment constraints for a GitHub Environment. +The traversable GH_CanDeployToEnvironment edge represents the ability for a repository, branch, repository role, or reviewer to satisfy the modeled deployment constraints for a GitHub Environment. This edge is computed from environment deployment branch policy, branch protection state, required reviewer behavior, and administrator bypass behavior. For environments without required reviewers, unrestricted environments emit repository and branch edges, protected-branch-only environments emit edges only for protected branches unless no branch protection rules exist, and custom branch policies emit edges only for matching branches. -When required reviewers are configured and self-review is allowed, the configured GH_User or GH_Team reviewer receives GH_CanDeployToEnvironment because that reviewer can satisfy the approval gate themselves. When prevent_self_review is enabled, no direct deploy edge is emitted for the reviewer because the required split-principal flow is not currently modeled. GH_ApprovesDeploymentTo remains non-traversable reviewer context in both cases. +When required reviewers are configured and self-review is allowed, a configured GH_User or GH_Team reviewer receives GH_CanDeployToEnvironment only when the same actor can also supply deployable code. For unrestricted environments this means the actor can create a branch in the repository. For protected-branch-only or custom branch policy environments this means the actor can write to an eligible branch under the existing GH_CanWriteBranch rules. + +Self-review alone is not sufficient for this edge. GH_ApprovesDeploymentTo remains the non-traversable representation of reviewer authority, while GH_CanDeployToEnvironment represents the combined ability to satisfy both the approval gate and the code-supply path. When prevent_self_review is enabled, no direct deploy edge is emitted for the reviewer because the required split-principal flow is not currently modeled. diff --git a/extension/schema.json b/extension/schema.json index 9e6bc02..0bdac7c 100644 --- a/extension/schema.json +++ b/extension/schema.json @@ -1042,7 +1042,7 @@ }, { "name": "GH_CanDeployToEnvironment", - "description": "[Computed] Repository, branch, repo role, or self-approving reviewer can deploy to this GitHub environment after evaluating deployment branch policy, reviewer gates, and admin bypass behavior", + "description": "[Computed] Repository, branch, repo role, or reviewer can deploy to this GitHub environment after evaluating deployment branch policy, reviewer gates, and admin bypass behavior; reviewer edges require both self-approval and a deployable code path", "is_traversable": true }, { diff --git a/src/openhound_github/lookup.py b/src/openhound_github/lookup.py index dfbe0a6..122b707 100644 --- a/src/openhound_github/lookup.py +++ b/src/openhound_github/lookup.py @@ -1,3 +1,4 @@ +import json from functools import lru_cache import duckdb @@ -584,6 +585,265 @@ def branches_for_repository(self, repository_node_id: str): [repository_node_id], ) + @lru_cache + def environment_branch_policy_names(self, environment_node_id: str): + return self._find_all_objects( + f""" + SELECT name + FROM {self.schema}.environment_branch_policies + WHERE environment_node_id = ? + """, + [environment_node_id], + ) + + @lru_cache + def reviewer_repo_role_assignments( + self, reviewer_node_id: str, reviewer_kind: str, repository_node_id: str + ): + reviewer_kind = reviewer_kind.lower() + return self._find_all_objects( + f""" + WITH RECURSIVE + repository_org(org_login) AS ( + SELECT org_login + FROM {self.schema}.repositories + WHERE node_id = ? + ), + seed_teams(team_id) AS ( + SELECT ? WHERE ? = 'team' + + UNION + + SELECT tm.team_id + FROM {self.schema}.team_members tm + WHERE ? = 'user' + AND tm.id = ? + ), + actor_teams(team_id) AS ( + SELECT team_id + FROM seed_teams + + UNION + + SELECT json_extract_string(t.parent_team, '$.id') + FROM {self.schema}.teams t + JOIN actor_teams actor_team ON t.id = actor_team.team_id + WHERE t.parent_team IS NOT NULL + AND json_extract_string(t.parent_team, '$.id') IS NOT NULL + ), + direct_repo_roles( + assignment_actor_id, + role_id, + role_name, + base_role, + role_permissions + ) AS ( + SELECT DISTINCT + rra.node_id, + rr.id, + rra.role_name, + coalesce(rra.base_role, rr.base_role), + rra.role_permissions + FROM {self.schema}.repo_role_assignments rra + LEFT JOIN {self.schema}.repo_roles rr + ON rr.repository_node_id = rra.repo_node_id + AND rr.name = rra.role_name + WHERE rra.repo_node_id = ? + AND ( + (lower(rra.assignee_type) = ? AND rra.node_id = ?) + OR ( + lower(rra.assignee_type) = 'team' + AND rra.node_id IN (SELECT team_id FROM actor_teams) + ) + ) + ), + actor_org_roles(assignment_actor_id, org_role_name, base_role) AS ( + SELECT + u.id, + CASE WHEN u.role = 'ADMIN' THEN 'owners' ELSE 'members' END, + org_role.base_role + FROM {self.schema}.users u + JOIN repository_org repo_org ON repo_org.org_login = u.org_login + JOIN {self.schema}.org_roles org_role + ON org_role.org_login = u.org_login + AND org_role.name = CASE + WHEN u.role = 'ADMIN' THEN 'owners' + ELSE 'members' + END + WHERE ? = 'user' + AND u.id = ? + + UNION + + SELECT + orm.node_id, + orm.org_role_name, + org_role.base_role + FROM {self.schema}.org_role_members orm + JOIN repository_org repo_org ON repo_org.org_login = orm.org_login + JOIN {self.schema}.org_roles org_role + ON org_role.org_login = orm.org_login + AND org_role.name = orm.org_role_name + WHERE ? = 'user' + AND orm.node_id = ? + + UNION + + SELECT + ort.node_id, + ort.org_role_name, + org_role.base_role + FROM {self.schema}.org_role_teams ort + JOIN repository_org repo_org ON repo_org.org_login = ort.org_login + JOIN {self.schema}.org_roles org_role + ON org_role.org_login = ort.org_login + AND org_role.name = ort.org_role_name + WHERE ort.node_id IN (SELECT team_id FROM actor_teams) + ), + org_repo_roles( + assignment_actor_id, + role_id, + role_name, + base_role, + role_permissions + ) AS ( + SELECT DISTINCT + actor_org_role.assignment_actor_id, + rr.id, + rr.name, + rr.base_role, + rr.permissions + FROM actor_org_roles actor_org_role + JOIN {self.schema}.repo_roles rr + ON rr.repository_node_id = ? + AND rr.name = actor_org_role.base_role + ) + SELECT * FROM direct_repo_roles + UNION + SELECT * FROM org_repo_roles + """, + [ + repository_node_id, + reviewer_node_id, + reviewer_kind, + reviewer_kind, + reviewer_node_id, + repository_node_id, + reviewer_kind, + reviewer_node_id, + reviewer_kind, + reviewer_node_id, + reviewer_kind, + reviewer_node_id, + repository_node_id, + ], + ) + + @staticmethod + def _role_permissions(raw_permissions) -> set[str]: + if raw_permissions is None: + return set() + if isinstance(raw_permissions, str): + try: + raw_permissions = json.loads(raw_permissions) + except json.JSONDecodeError: + return set() + if not isinstance(raw_permissions, list): + return set() + return {str(permission) for permission in raw_permissions} + + @lru_cache + def reviewer_deployment_path( + self, + reviewer_node_id: str, + reviewer_kind: str, + repository_node_id: str, + eligible_branch_ids: tuple[str, ...], + allow_create_branch: bool, + ) -> tuple[str, str | None] | None: + if not self.repository_default_branch_collected(repository_node_id): + return None + + eligible_branches = set(eligible_branch_ids) + write_roles = {"write", "maintain", "admin"} + bypass_roles = {"maintain"} + + for ( + assignment_actor_id, + role_id, + role_name, + base_role, + raw_permissions, + ) in self.reviewer_repo_role_assignments( + reviewer_node_id, reviewer_kind, repository_node_id + ): + permissions = self._role_permissions(raw_permissions) + has_write_access = role_name in write_roles or base_role in write_roles + if not has_write_access: + continue + + if ( + allow_create_branch + and role_id is not None + and self.role_can_create_branch(role_id, repository_node_id) + ): + return ("create_branch", None) + + writable_branches = { + branch_id + for (branch_id,) in self.unprotected_branches(repository_node_id) + } + + has_push_protected_branch = ( + ("push_protected_branch" in permissions and base_role in write_roles) + or role_name in bypass_roles + or base_role in bypass_roles + ) + has_bypass_branch_protection = ( + "bypass_branch_protection" in permissions and base_role in write_roles + ) + + if role_name == "admin" or base_role == "admin": + writable_branches.update( + branch_id + for (branch_id,) in self._write_admin_bypass(repository_node_id) + ) + if has_push_protected_branch: + writable_branches.update( + branch_id + for (branch_id,) in self._write_push_restricted_branch_bypass( + repository_node_id + ) + ) + if has_bypass_branch_protection: + writable_branches.update( + branch_id + for (branch_id,) in self._write_branch_protection_bypass( + repository_node_id + ) + ) + if has_push_protected_branch and has_bypass_branch_protection: + writable_branches.update( + branch_id + for (branch_id,) in self._write_combined_bypass(repository_node_id) + ) + + writable_branches.update( + branch_id + for (branch_id,) in self.actor_gate_bypass( + assignment_actor_id, + repository_node_id, + has_bypass_branch_protection, + has_push_protected_branch, + ) + ) + + for branch_id in eligible_branch_ids: + if branch_id in writable_branches and branch_id in eligible_branches: + return ("write_branch", branch_id) + + return None + @lru_cache def members_can_fork_private_repositories(self, org_login: str): return self._find_all_objects( diff --git a/src/openhound_github/main.py b/src/openhound_github/main.py index 9cc182d..708bcdf 100644 --- a/src/openhound_github/main.py +++ b/src/openhound_github/main.py @@ -59,6 +59,9 @@ def preproc(ctx: PreProcContext): "repo_role_assignments": "repo_role_assignments", "branches": "branches", "repo_roles": "repo_roles", + "users": "users", + "teams": "teams", + "team_members": "team_members", "saml_provider": "saml_provider", "applications": "applications", "enterprise": "enterprise", @@ -67,8 +70,11 @@ def preproc(ctx: PreProcContext): "enterprise_runner_group_organizations": "enterprise_runner_group_organizations", "enterprise_runner_group_memberships": "enterprise_runner_group_memberships", "org_roles": "org_roles", + "org_role_members": "org_role_members", + "org_role_teams": "org_role_teams", "projected_enterprise_teams": "projected_enterprise_teams", "environments": "environments", + "environment_branch_policies": "environment_branch_policies", "environment_secrets": "environment_secrets", "environment_variables": "environment_variables", "organization_secrets": "organization_secrets", diff --git a/src/openhound_github/models/environment.py b/src/openhound_github/models/environment.py index cf26b82..078cb07 100644 --- a/src/openhound_github/models/environment.py +++ b/src/openhound_github/models/environment.py @@ -10,6 +10,9 @@ from openhound_github.kinds import edges as ek from openhound_github.kinds import nodes as nk from openhound_github.main import app +from openhound_github.models.environment_branch_policy import ( + matches_environment_branch_policy, +) class DeploymentBranchPolicy(BaseModel): @@ -142,14 +145,14 @@ class GHEnvironmentProperties(GHNodeProperties): start=nk.USER, end=nk.ENVIRONMENT, kind=ek.CAN_DEPLOY_TO_ENVIRONMENT, - description="Reviewer user can self-approve deployment to environment", + description="Reviewer user can self-approve and supply deployable code under current branch policy", traversable=True, ), EdgeDef( start=nk.TEAM, end=nk.ENVIRONMENT, kind=ek.CAN_DEPLOY_TO_ENVIRONMENT, - description="Reviewer team can self-approve deployment to environment", + description="Reviewer team can self-approve and supply deployable code under current branch policy", traversable=True, ), EdgeDef( @@ -240,6 +243,55 @@ def source_deployment_edges_allowed(self) -> bool: def reviewer_self_deployment_edges_allowed(self) -> bool: return self.required_reviewers and not self.prevent_self_review + @property + def _reviewer_eligible_branch_ids(self) -> tuple[str, ...]: + branches = self._lookup.branches_for_repository(self.repository_node_id) + + if self.has_custom_branch_policies: + policy_names = { + policy_name + for (policy_name,) in self._lookup.environment_branch_policy_names( + self.node_id + ) + } + return tuple( + branch_id + for branch_id, branch_name, protected in branches + if any( + matches_environment_branch_policy(branch_name, policy_name) + for policy_name in policy_names + ) + and ( + not ( + self.deployment_branch_policy + and self.deployment_branch_policy.protected_branches + ) + or protected + ) + ) + + if ( + self.deployment_branch_policy + and self.deployment_branch_policy.protected_branches + ): + protected_branch_ids = tuple( + branch_id + for (branch_id,) in self._lookup.branches_with_bpr( + self.repository_node_id + ) + ) + if protected_branch_ids: + return protected_branch_ids + + return tuple(branch_id for branch_id, _branch_name, _protected in branches) + + @property + def _reviewer_can_create_branch_for_deployment(self) -> bool: + return not self.has_custom_branch_policies and not ( + self.deployment_branch_policy + and self.deployment_branch_policy.protected_branches + ) + @property def as_node(self) -> GHNode: eid = self.node_id @@ -313,16 +365,89 @@ def _admin_bypass_query(self) -> str: f"RETURN p" ) - def _reviewer_can_deploy_query( + def _reviewer_can_create_branch_query( self, reviewer_node_id: str, reviewer_kind: str ) -> str: return ( - f"MATCH p=(:{reviewer_kind} {{node_id:'{reviewer_node_id}'}})" - f"-[:GH_ApprovesDeploymentTo]->" + f"MATCH p=(actor:{reviewer_kind} {{node_id:'{reviewer_node_id}'}})" + f"-[:GH_HasRole|GH_HasBaseRole|GH_MemberOf*1..]->" + f"(:GH_RepoRole)-[:GH_CanCreateBranch]->" + f"(repo:GH_Repository {{node_id:'{self.repository_node_id}'}}) " + f"MATCH p1=(actor)-[:GH_ApprovesDeploymentTo]->" f"(env:GH_Environment {{node_id:'{self.node_id}'}}) " + f"MATCH p2=(repo)-[:GH_Contains]->(env) " f"WHERE env.required_reviewers = true " f"AND coalesce(env.prevent_self_review, false) = false " - f"RETURN p" + f"AND coalesce(env.custom_branch_policies, false) = false " + f"AND coalesce(env.protected_branches, false) = false " + f"RETURN p, p1, p2" + ) + + def _reviewer_can_write_branch_query( + self, reviewer_node_id: str, reviewer_kind: str, branch_id: str + ) -> str: + actor_path = ( + f"MATCH p=(actor:{reviewer_kind} {{node_id:'{reviewer_node_id}'}})" + f"-[:GH_HasRole|GH_HasBaseRole|GH_MemberOf|GH_CanWriteBranch*1..]->" + f"(branch:GH_Branch {{node_id:'{branch_id}'}}) " + f"MATCH p1=(actor)-[:GH_ApprovesDeploymentTo]->" + f"(env:GH_Environment {{node_id:'{self.node_id}'}}) " + ) + reviewer_policy = ( + "WHERE env.required_reviewers = true " + "AND coalesce(env.prevent_self_review, false) = false " + ) + + if self.has_custom_branch_policies: + return ( + actor_path + + "MATCH p2=(branch)-[:GH_MatchesEnvironmentPolicy]->" + "(:GH_EnvironmentBranchPolicy)<-[:GH_Contains]-(env) " + + reviewer_policy + + "AND env.custom_branch_policies = true " + + "RETURN p, p1, p2" + ) + + if ( + self.deployment_branch_policy + and self.deployment_branch_policy.protected_branches + ): + if self._lookup.branches_with_bpr(self.repository_node_id): + return ( + actor_path + + f"MATCH p2=(repo:GH_Repository {{node_id:'{self.repository_node_id}'}})" + f"-[:GH_Contains]->(branch)<-[:GH_ProtectedBy]-(:GH_BranchProtectionRule) " + f"MATCH p3=(repo)-[:GH_Contains]->(env) " + + reviewer_policy + + "AND env.protected_branches = true " + + "AND coalesce(env.custom_branch_policies, false) = false " + + "RETURN p, p1, p2, p3" + ) + + return ( + actor_path + + f"MATCH p2=(repo:GH_Repository {{node_id:'{self.repository_node_id}'}})" + f"-[:GH_Contains]->(branch) " + f"MATCH p3=(repo)-[:GH_Contains]->(env) " + + reviewer_policy + + "AND env.protected_branches = true " + + "AND coalesce(env.custom_branch_policies, false) = false " + + "AND NOT EXISTS { " + + "MATCH (repo)-[:GH_Contains]->(:GH_Branch)" + + "<-[:GH_ProtectedBy]-(:GH_BranchProtectionRule) " + + "} " + + "RETURN p, p1, p2, p3" + ) + + return ( + actor_path + + f"MATCH p2=(repo:GH_Repository {{node_id:'{self.repository_node_id}'}})" + f"-[:GH_Contains]->(branch) " + f"MATCH p3=(repo)-[:GH_Contains]->(env) " + + reviewer_policy + + "AND coalesce(env.custom_branch_policies, false) = false " + + "AND coalesce(env.protected_branches, false) = false " + + "RETURN p, p1, p2, p3" ) def _protected_branches_fallback_repo_query(self) -> str: @@ -480,6 +605,26 @@ def edges(self): if self.reviewer_self_deployment_edges_allowed: reviewer_kind = nk.USER if reviewer_type == "user" else nk.TEAM + deployment_path = self._lookup.reviewer_deployment_path( + reviewer_node_id, + reviewer_type, + self.repository_node_id, + self._reviewer_eligible_branch_ids, + self._reviewer_can_create_branch_for_deployment, + ) + if deployment_path is None: + continue + + path_type, branch_id = deployment_path + query_composition = ( + self._reviewer_can_create_branch_query( + reviewer_node_id, reviewer_kind + ) + if path_type == "create_branch" + else self._reviewer_can_write_branch_query( + reviewer_node_id, reviewer_kind, branch_id or "" + ) + ) yield Edge( kind=ek.CAN_DEPLOY_TO_ENVIRONMENT, start=EdgePath(value=reviewer_node_id, match_by="id"), @@ -487,8 +632,6 @@ def edges(self): properties=GHEdgeProperties( traversable=True, composed=True, - query_composition=self._reviewer_can_deploy_query( - reviewer_node_id, reviewer_kind - ), + query_composition=query_composition, ), ) diff --git a/src/openhound_github/models/environment_branch_policy.py b/src/openhound_github/models/environment_branch_policy.py index 0aee87c..05a40e1 100644 --- a/src/openhound_github/models/environment_branch_policy.py +++ b/src/openhound_github/models/environment_branch_policy.py @@ -11,6 +11,14 @@ # from openhound_github.helpers import _b64 + +def matches_environment_branch_policy(branch_name: str, policy_name: str) -> bool: + return PurePosixPath(f"/{branch_name}").full_match( + f"/{policy_name}", + case_sensitive=True, + ) + + @dataclass class GHEnvironmentBranchPolicyProperties(GHNodeProperties): environment_name: str | None = None @@ -88,10 +96,7 @@ def environment_required_reviewers(self) -> bool: return required_reviewers def matches_branch(self, branch_name: str) -> bool: - return PurePosixPath(f"/{branch_name}").full_match( - f"/{self.name}", - case_sensitive=True, - ) + return matches_environment_branch_policy(branch_name, self.name) @property def as_node(self) -> GHNode: diff --git a/src/openhound_github/transforms.py b/src/openhound_github/transforms.py index a86ee9f..f31d29e 100644 --- a/src/openhound_github/transforms.py +++ b/src/openhound_github/transforms.py @@ -4,11 +4,12 @@ def ensure_optional_input_tables( con: duckdb.DuckDBPyConnection, schema: str = "github" ) -> None: - """Create typed empty tables for zero-row branch-policy inputs. + """Create typed empty tables for zero-row derived-edge inputs. DLT omits resources that yield no rows. Enterprise GitHub App collection can - legitimately have no branches, branch-protection rules, or repository roles, - while the derived branch transforms still need stable input schemas. + legitimately have no branches, branch-protection rules, role assignments, or + environment branch policies while the derived transforms still need stable + input schemas. """ con.execute(f""" CREATE TABLE IF NOT EXISTS {schema}.branches ( @@ -35,9 +36,46 @@ def ensure_optional_input_tables( ); CREATE TABLE IF NOT EXISTS {schema}.repo_roles ( id BIGINT, + name VARCHAR, + base_role VARCHAR, repository_node_id VARCHAR, permissions JSON ); + CREATE TABLE IF NOT EXISTS {schema}.repo_role_assignments ( + node_id VARCHAR, + assignee_type VARCHAR, + repo_node_id VARCHAR, + role_name VARCHAR, + base_role VARCHAR, + role_permissions JSON + ); + CREATE TABLE IF NOT EXISTS {schema}.users ( + id VARCHAR, + role VARCHAR, + org_login VARCHAR + ); + CREATE TABLE IF NOT EXISTS {schema}.teams ( + id VARCHAR, + parent_team JSON + ); + CREATE TABLE IF NOT EXISTS {schema}.team_members ( + team_id VARCHAR, + id VARCHAR + ); + CREATE TABLE IF NOT EXISTS {schema}.org_role_members ( + node_id VARCHAR, + org_role_name VARCHAR, + org_login VARCHAR + ); + CREATE TABLE IF NOT EXISTS {schema}.org_role_teams ( + node_id VARCHAR, + org_role_name VARCHAR, + org_login VARCHAR + ); + CREATE TABLE IF NOT EXISTS {schema}.environment_branch_policies ( + environment_node_id VARCHAR, + name VARCHAR + ); CREATE TABLE IF NOT EXISTS {schema}.organization_variables ( name VARCHAR, org_login VARCHAR, @@ -105,9 +143,62 @@ def ensure_optional_input_tables( ALTER TABLE {schema}.branch_protection_rules ADD COLUMN IF NOT EXISTS blocks_creations BOOLEAN; + ALTER TABLE {schema}.repo_roles + ADD COLUMN IF NOT EXISTS name VARCHAR; + ALTER TABLE {schema}.repo_roles + ADD COLUMN IF NOT EXISTS base_role VARCHAR; ALTER TABLE {schema}.repo_roles ADD COLUMN IF NOT EXISTS permissions JSON; + ALTER TABLE {schema}.repo_role_assignments + ADD COLUMN IF NOT EXISTS node_id VARCHAR; + ALTER TABLE {schema}.repo_role_assignments + ADD COLUMN IF NOT EXISTS assignee_type VARCHAR; + ALTER TABLE {schema}.repo_role_assignments + ADD COLUMN IF NOT EXISTS repo_node_id VARCHAR; + ALTER TABLE {schema}.repo_role_assignments + ADD COLUMN IF NOT EXISTS role_name VARCHAR; + ALTER TABLE {schema}.repo_role_assignments + ADD COLUMN IF NOT EXISTS base_role VARCHAR; + ALTER TABLE {schema}.repo_role_assignments + ADD COLUMN IF NOT EXISTS role_permissions JSON; + + ALTER TABLE {schema}.users + ADD COLUMN IF NOT EXISTS id VARCHAR; + ALTER TABLE {schema}.users + ADD COLUMN IF NOT EXISTS role VARCHAR; + ALTER TABLE {schema}.users + ADD COLUMN IF NOT EXISTS org_login VARCHAR; + + ALTER TABLE {schema}.teams + ADD COLUMN IF NOT EXISTS id VARCHAR; + ALTER TABLE {schema}.teams + ADD COLUMN IF NOT EXISTS parent_team JSON; + + ALTER TABLE {schema}.team_members + ADD COLUMN IF NOT EXISTS team_id VARCHAR; + ALTER TABLE {schema}.team_members + ADD COLUMN IF NOT EXISTS id VARCHAR; + + ALTER TABLE {schema}.org_role_members + ADD COLUMN IF NOT EXISTS node_id VARCHAR; + ALTER TABLE {schema}.org_role_members + ADD COLUMN IF NOT EXISTS org_role_name VARCHAR; + ALTER TABLE {schema}.org_role_members + ADD COLUMN IF NOT EXISTS org_login VARCHAR; + + ALTER TABLE {schema}.org_role_teams + ADD COLUMN IF NOT EXISTS node_id VARCHAR; + ALTER TABLE {schema}.org_role_teams + ADD COLUMN IF NOT EXISTS org_role_name VARCHAR; + ALTER TABLE {schema}.org_role_teams + ADD COLUMN IF NOT EXISTS org_login VARCHAR; + + ALTER TABLE {schema}.environment_branch_policies + ADD COLUMN IF NOT EXISTS environment_node_id VARCHAR; + ALTER TABLE {schema}.environment_branch_policies + ADD COLUMN IF NOT EXISTS name VARCHAR; + ALTER TABLE {schema}.organization_variables ADD COLUMN IF NOT EXISTS name VARCHAR; ALTER TABLE {schema}.organization_variables diff --git a/tests/test_environment_model.py b/tests/test_environment_model.py index 42c148c..0084843 100644 --- a/tests/test_environment_model.py +++ b/tests/test_environment_model.py @@ -87,6 +87,23 @@ def _make_environment() -> Environment: ) lookup = MagicMock() lookup.org_id_for_login.return_value = "O_123" + lookup.branches_with_bpr.return_value = [] + lookup.environment_branch_policy_names.return_value = [] + + def reviewer_deployment_path( + _reviewer_node_id, + _reviewer_type, + _repository_node_id, + eligible_branch_ids, + allow_create_branch, + ): + if allow_create_branch: + return ("create_branch", None) + if eligible_branch_ids: + return ("write_branch", eligible_branch_ids[0]) + return None + + lookup.reviewer_deployment_path.side_effect = reviewer_deployment_path env._lookup = lookup return env @@ -172,6 +189,87 @@ def test_environment_with_self_review_emits_only_reviewer_deploy_edges() -> None in edge.properties.query_composition for edge in deploy_edges ) + assert all( + "GH_CanCreateBranch" in edge.properties.query_composition + for edge in deploy_edges + ) + + +def test_environment_reviewer_without_deployable_path_only_emits_approval_edge() -> None: + env = _make_unrestricted_environment() + env._lookup.reviewer_deployment_path.return_value = None + env._lookup.reviewer_deployment_path.side_effect = None + + edges = list(env.edges) + + assert { + edge.start.value + for edge in edges + if edge.kind == ek.APPROVES_DEPLOYMENT_TO + } == {"MDQ6VXNlcjE=", "MDQ6VGVhbTE="} + assert _deploy_edges(env) == [] + + +def test_environment_reviewer_protected_branch_path_requires_eligible_branch() -> None: + env = _make_environment() + env.deployment_branch_policy = DeploymentBranchPolicy( + protected_branches=True, + custom_branch_policies=False, + ) + env._lookup.branches_for_repository.return_value = [ + ("B_main", "main", True), + ("B_release", "release/v1", False), + ] + env._lookup.branches_with_bpr.return_value = [("B_main",)] + env._lookup.reviewer_deployment_path.return_value = ("write_branch", "B_main") + env._lookup.reviewer_deployment_path.side_effect = None + + deploy_edges = _deploy_edges(env) + + assert {edge.start.value for edge in deploy_edges} == { + "MDQ6VXNlcjE=", + "MDQ6VGVhbTE=", + } + assert all( + "GH_ProtectedBy" in edge.properties.query_composition + for edge in deploy_edges + ) + env._lookup.reviewer_deployment_path.assert_any_call( + "MDQ6VXNlcjE=", + "user", + "R_123", + ("B_main",), + False, + ) + + +def test_environment_reviewer_custom_policy_uses_matching_branches() -> None: + env = _make_environment() + env._lookup.branches_for_repository.return_value = [ + ("B_main", "main", False), + ("B_release", "release/v1", False), + ] + env._lookup.environment_branch_policy_names.return_value = [("release/*",)] + env._lookup.reviewer_deployment_path.return_value = ("write_branch", "B_release") + env._lookup.reviewer_deployment_path.side_effect = None + + deploy_edges = _deploy_edges(env) + + assert {edge.start.value for edge in deploy_edges} == { + "MDQ6VXNlcjE=", + "MDQ6VGVhbTE=", + } + assert all( + "GH_MatchesEnvironmentPolicy" in edge.properties.query_composition + for edge in deploy_edges + ) + env._lookup.reviewer_deployment_path.assert_any_call( + "MDQ6VXNlcjE=", + "user", + "R_123", + ("B_release",), + False, + ) def test_environment_with_prevent_self_review_emits_no_direct_deploy_edges() -> None: diff --git a/tests/test_environment_reviewer_lookup.py b/tests/test_environment_reviewer_lookup.py new file mode 100644 index 0000000..5ce7b75 --- /dev/null +++ b/tests/test_environment_reviewer_lookup.py @@ -0,0 +1,172 @@ +import duckdb + +from openhound_github.lookup import GithubLookup + + +def _lookup() -> GithubLookup: + connection = duckdb.connect(":memory:") + connection.execute("CREATE SCHEMA github") + connection.execute( + """ + CREATE TABLE github.repositories ( + node_id VARCHAR, + default_branch VARCHAR, + org_login VARCHAR + ); + CREATE TABLE github.branches ( + id VARCHAR, + name VARCHAR, + repository_node_id VARCHAR, + branch_protection_rule JSON + ); + CREATE TABLE github.repo_roles ( + id BIGINT, + name VARCHAR, + base_role VARCHAR, + repository_node_id VARCHAR, + permissions JSON + ); + CREATE TABLE github.repo_role_assignments ( + node_id VARCHAR, + assignee_type VARCHAR, + repo_node_id VARCHAR, + role_name VARCHAR, + base_role VARCHAR, + role_permissions JSON + ); + CREATE TABLE github.teams ( + id VARCHAR, + parent_team JSON + ); + CREATE TABLE github.team_members ( + team_id VARCHAR, + id VARCHAR + ); + CREATE TABLE github.users ( + id VARCHAR, + role VARCHAR, + org_login VARCHAR + ); + CREATE TABLE github.org_roles ( + name VARCHAR, + base_role VARCHAR, + org_login VARCHAR + ); + CREATE TABLE github.org_role_members ( + node_id VARCHAR, + org_role_name VARCHAR, + org_login VARCHAR + ); + CREATE TABLE github.org_role_teams ( + node_id VARCHAR, + org_role_name VARCHAR, + org_login VARCHAR + ); + CREATE TABLE github.role_can_create_branch ( + id BIGINT, + repository_node_id VARCHAR + ); + CREATE TABLE github.unprotected_branches ( + id VARCHAR, + repository_node_id VARCHAR + ); + CREATE TABLE github.branch_bpr ( + id VARCHAR, + repository_node_id VARCHAR, + requires_approving_reviews BOOLEAN, + lock_branch BOOLEAN, + restricts_pushes BOOLEAN, + is_admin_enforced BOOLEAN + ); + CREATE TABLE github.actor_branch_gates ( + actor_id VARCHAR, + branch_id VARCHAR, + repository_node_id VARCHAR, + has_push_allowance BOOLEAN, + has_pr_allowance BOOLEAN, + requires_approving_reviews BOOLEAN, + lock_branch BOOLEAN, + restricts_pushes BOOLEAN, + is_admin_enforced BOOLEAN + ); + """ + ) + connection.execute( + """ + INSERT INTO github.repositories VALUES ('REPO_1', 'main', 'acme'); + INSERT INTO github.branches VALUES + ('B_main', 'main', 'REPO_1', NULL), + ('B_release', 'release', 'REPO_1', '{"id":"BPR_1"}'); + INSERT INTO github.repo_roles VALUES (2, 'write', NULL, 'REPO_1', '[]'); + INSERT INTO github.repo_role_assignments VALUES + ('TEAM_PARENT', 'team', 'REPO_1', 'write', NULL, '[]'); + INSERT INTO github.teams VALUES + ('TEAM_CHILD', '{"id":"TEAM_PARENT"}'), + ('TEAM_PARENT', NULL), + ('TEAM_ORG_CHILD', '{"id":"TEAM_ORG_PARENT"}'), + ('TEAM_ORG_PARENT', NULL); + INSERT INTO github.team_members VALUES ('TEAM_CHILD', 'USER_1'); + INSERT INTO github.team_members VALUES ('TEAM_ORG_CHILD', 'USER_ORG_TEAM'); + INSERT INTO github.users VALUES + ('USER_MEMBER', 'MEMBER', 'acme'), + ('USER_ORG_TEAM', 'MEMBER', 'acme'); + INSERT INTO github.org_roles VALUES + ('members', 'write', 'acme'), + ('deployers', 'write', 'acme'); + INSERT INTO github.org_role_teams VALUES ('TEAM_ORG_PARENT', 'deployers', 'acme'); + INSERT INTO github.role_can_create_branch VALUES (2, 'REPO_1'); + INSERT INTO github.unprotected_branches VALUES ('B_main', 'REPO_1'); + """ + ) + return GithubLookup(connection) + + +def test_reviewer_deployment_path_unrolls_user_team_membership_for_branch_creation() -> None: + lookup = _lookup() + + assert lookup.reviewer_deployment_path( + "USER_1", + "user", + "REPO_1", + ("B_main",), + True, + ) == ("create_branch", None) + + +def test_reviewer_deployment_path_requires_an_eligible_writable_branch() -> None: + lookup = _lookup() + + assert ( + lookup.reviewer_deployment_path( + "USER_1", + "user", + "REPO_1", + ("B_release",), + False, + ) + is None + ) + + +def test_reviewer_deployment_path_unrolls_default_org_role() -> None: + lookup = _lookup() + + assert lookup.reviewer_deployment_path( + "USER_MEMBER", + "user", + "REPO_1", + ("B_main",), + True, + ) == ("create_branch", None) + + +def test_reviewer_deployment_path_unrolls_custom_org_role_through_parent_team() -> None: + lookup = _lookup() + + assert lookup.reviewer_deployment_path( + "USER_ORG_TEAM", + "user", + "REPO_1", + ("B_main",), + True, + ) == ("create_branch", None) From 84c6a6da46ca02bc94274914b7038d2f7bd47100 Mon Sep 17 00:00:00 2001 From: Jared Atkinson Date: Wed, 19 Aug 2026 09:36:24 -0700 Subject: [PATCH 2/3] BED-9371: preserve protected branch evidence for custom policies When an environment enables both custom branch policies and protected-branch-only deployment, include the GH_ProtectedBy hop and protected_branches predicate in reviewer deployment composition queries. Also make custom-only queries explicitly require protected_branches=false and add coverage for both combinations so eligible branch selection and composed evidence stay aligned. --- src/openhound_github/models/environment.py | 34 ++++++++++---- tests/test_environment_model.py | 54 ++++++++++++++++++++++ 2 files changed, 79 insertions(+), 9 deletions(-) diff --git a/src/openhound_github/models/environment.py b/src/openhound_github/models/environment.py index 078cb07..2c1fb9e 100644 --- a/src/openhound_github/models/environment.py +++ b/src/openhound_github/models/environment.py @@ -248,6 +248,10 @@ def _reviewer_eligible_branch_ids(self) -> tuple[str, ...]: branches = self._lookup.branches_for_repository(self.repository_node_id) if self.has_custom_branch_policies: + protected_branches_only = bool( + self.deployment_branch_policy + and self.deployment_branch_policy.protected_branches + ) policy_names = { policy_name for (policy_name,) in self._lookup.environment_branch_policy_names( @@ -261,13 +265,7 @@ def _reviewer_eligible_branch_ids(self) -> tuple[str, ...]: matches_environment_branch_policy(branch_name, policy_name) for policy_name in policy_names ) - and ( - not ( - self.deployment_branch_policy - and self.deployment_branch_policy.protected_branches - ) - or protected - ) + and (not protected_branches_only or protected) ) if ( @@ -399,12 +397,30 @@ def _reviewer_can_write_branch_query( ) if self.has_custom_branch_policies: + custom_policy_path = ( + "MATCH p2=(branch)-[:GH_MatchesEnvironmentPolicy]->" + "(:GH_EnvironmentBranchPolicy)<-[:GH_Contains]-(env) " + ) + if ( + self.deployment_branch_policy + and self.deployment_branch_policy.protected_branches + ): + return ( + actor_path + + custom_policy_path + + "MATCH p3=(branch)<-[:GH_ProtectedBy]-(:GH_BranchProtectionRule) " + + reviewer_policy + + "AND env.custom_branch_policies = true " + + "AND env.protected_branches = true " + + "RETURN p, p1, p2, p3" + ) + return ( actor_path - + "MATCH p2=(branch)-[:GH_MatchesEnvironmentPolicy]->" - "(:GH_EnvironmentBranchPolicy)<-[:GH_Contains]-(env) " + + custom_policy_path + reviewer_policy + "AND env.custom_branch_policies = true " + + "AND coalesce(env.protected_branches, false) = false " + "RETURN p, p1, p2" ) diff --git a/tests/test_environment_model.py b/tests/test_environment_model.py index 0084843..79ad078 100644 --- a/tests/test_environment_model.py +++ b/tests/test_environment_model.py @@ -263,6 +263,15 @@ def test_environment_reviewer_custom_policy_uses_matching_branches() -> None: "GH_MatchesEnvironmentPolicy" in edge.properties.query_composition for edge in deploy_edges ) + assert all( + "coalesce(env.protected_branches, false) = false" + in edge.properties.query_composition + for edge in deploy_edges + ) + assert all( + "GH_ProtectedBy" not in edge.properties.query_composition + for edge in deploy_edges + ) env._lookup.reviewer_deployment_path.assert_any_call( "MDQ6VXNlcjE=", "user", @@ -272,6 +281,51 @@ def test_environment_reviewer_custom_policy_uses_matching_branches() -> None: ) +def test_environment_reviewer_custom_policy_with_protected_branches_uses_matching_protected_branches() -> None: + env = _make_environment() + env.deployment_branch_policy = DeploymentBranchPolicy( + protected_branches=True, + custom_branch_policies=True, + ) + env._lookup.branches_for_repository.return_value = [ + ("B_main", "main", True), + ("B_release_unprotected", "release/v1", False), + ("B_release_protected", "release/v2", True), + ] + env._lookup.environment_branch_policy_names.return_value = [("release/*",)] + env._lookup.reviewer_deployment_path.return_value = ( + "write_branch", + "B_release_protected", + ) + env._lookup.reviewer_deployment_path.side_effect = None + + deploy_edges = _deploy_edges(env) + + assert {edge.start.value for edge in deploy_edges} == { + "MDQ6VXNlcjE=", + "MDQ6VGVhbTE=", + } + assert all( + "GH_MatchesEnvironmentPolicy" in edge.properties.query_composition + for edge in deploy_edges + ) + assert all( + "GH_ProtectedBy" in edge.properties.query_composition + for edge in deploy_edges + ) + assert all( + "env.protected_branches = true" in edge.properties.query_composition + for edge in deploy_edges + ) + env._lookup.reviewer_deployment_path.assert_any_call( + "MDQ6VXNlcjE=", + "user", + "R_123", + ("B_release_protected",), + False, + ) + + def test_environment_with_prevent_self_review_emits_no_direct_deploy_edges() -> None: env = _make_unrestricted_environment() env.protection_rules[1].prevent_self_review = True From 67d2af0e3c35f0d8c7a9cc080c975363f0fe7e7d Mon Sep 17 00:00:00 2001 From: Jared Atkinson Date: Wed, 19 Aug 2026 09:38:51 -0700 Subject: [PATCH 3/3] BED-9371: tolerate missing org role lookup input Create an empty org_roles lookup table during preprocessing when the resource yields no rows so reviewer role resolution can still evaluate direct repository-role assignments. Add regression coverage proving direct reviewer deployment paths are preserved when org_roles is absent. --- src/openhound_github/transforms.py | 24 +++++++++++++++++++++++ tests/test_environment_reviewer_lookup.py | 21 ++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/src/openhound_github/transforms.py b/src/openhound_github/transforms.py index f31d29e..006fb2e 100644 --- a/src/openhound_github/transforms.py +++ b/src/openhound_github/transforms.py @@ -62,6 +62,15 @@ def ensure_optional_input_tables( team_id VARCHAR, id VARCHAR ); + CREATE TABLE IF NOT EXISTS {schema}.org_roles ( + id BIGINT, + name VARCHAR, + type VARCHAR, + base_role VARCHAR, + permissions JSON, + org_node_id VARCHAR, + org_login VARCHAR + ); CREATE TABLE IF NOT EXISTS {schema}.org_role_members ( node_id VARCHAR, org_role_name VARCHAR, @@ -180,6 +189,21 @@ def ensure_optional_input_tables( ALTER TABLE {schema}.team_members ADD COLUMN IF NOT EXISTS id VARCHAR; + ALTER TABLE {schema}.org_roles + ADD COLUMN IF NOT EXISTS id BIGINT; + ALTER TABLE {schema}.org_roles + ADD COLUMN IF NOT EXISTS name VARCHAR; + ALTER TABLE {schema}.org_roles + ADD COLUMN IF NOT EXISTS type VARCHAR; + ALTER TABLE {schema}.org_roles + ADD COLUMN IF NOT EXISTS base_role VARCHAR; + ALTER TABLE {schema}.org_roles + ADD COLUMN IF NOT EXISTS permissions JSON; + ALTER TABLE {schema}.org_roles + ADD COLUMN IF NOT EXISTS org_node_id VARCHAR; + ALTER TABLE {schema}.org_roles + ADD COLUMN IF NOT EXISTS org_login VARCHAR; + ALTER TABLE {schema}.org_role_members ADD COLUMN IF NOT EXISTS node_id VARCHAR; ALTER TABLE {schema}.org_role_members diff --git a/tests/test_environment_reviewer_lookup.py b/tests/test_environment_reviewer_lookup.py index 5ce7b75..d45cdcb 100644 --- a/tests/test_environment_reviewer_lookup.py +++ b/tests/test_environment_reviewer_lookup.py @@ -1,6 +1,7 @@ import duckdb from openhound_github.lookup import GithubLookup +from openhound_github.transforms import ensure_optional_input_tables def _lookup() -> GithubLookup: @@ -170,3 +171,23 @@ def test_reviewer_deployment_path_unrolls_custom_org_role_through_parent_team() ("B_main",), True, ) == ("create_branch", None) + + +def test_reviewer_deployment_path_preserves_direct_assignment_without_org_roles() -> None: + lookup = _lookup() + lookup.client.execute("DROP TABLE github.org_roles") + lookup.client.execute( + """ + INSERT INTO github.repo_role_assignments VALUES + ('USER_DIRECT', 'user', 'REPO_1', 'write', NULL, '[]') + """ + ) + ensure_optional_input_tables(lookup.client) + + assert lookup.reviewer_deployment_path( + "USER_DIRECT", + "user", + "REPO_1", + ("B_main",), + True, + ) == ("create_branch", None)