From 5b34ed669a48388ce3b303353036f3cbab2755cf Mon Sep 17 00:00:00 2001 From: Elliot Mackenzie <6545046+barfle@users.noreply.github.com> Date: Mon, 29 Jun 2026 17:08:11 +1000 Subject: [PATCH 1/4] Harden branch protection baselines for public release safety. Document and codify stricter PR and CI requirements so public-facing changes cannot bypass review or quality gates. --- .github/BRANCH_PROTECTION_RULESETS.md | 86 +++++++++++++++++++++++++++ .github/ruleset-main.json | 40 +++++++++++++ .github/ruleset-release.json | 40 +++++++++++++ 3 files changed, 166 insertions(+) create mode 100644 .github/BRANCH_PROTECTION_RULESETS.md create mode 100644 .github/ruleset-main.json create mode 100644 .github/ruleset-release.json diff --git a/.github/BRANCH_PROTECTION_RULESETS.md b/.github/BRANCH_PROTECTION_RULESETS.md new file mode 100644 index 0000000..ceb5f4f --- /dev/null +++ b/.github/BRANCH_PROTECTION_RULESETS.md @@ -0,0 +1,86 @@ +# Branch protection rulesets + +This repository uses two GitHub branch protection rulesets. + +## Per-repo rules + +1. **Default branch (main)** + - Target: branch name `main`. + - Require a pull request with **1 approval**, stale review dismissal, and resolved review threads. + - Require status checks: **Analyze (python)**, **Unit tests (3.11)**, **Unit tests (3.12)**, **Compile + help smoke (macos-latest, 3.11)**, **Compile + help smoke (windows-latest, 3.11)**, **No build artifacts tracked**. + - Require linear history. + - Block force pushes and branch deletion. + - Bypass: repo admins only (default). + +2. **Release branches** + - Target: branch pattern `release/*`. + - Same rules as above (PR + approval + thread resolution + strict required checks + linear history + no force push + no deletion). + +## Branch vs repo deletion + +- **Branches (e.g. main)**: The **deletion** rule in these rulesets protects the targeted branches. Only users with bypass permission (e.g. repo admins) can delete `main` or `release/*`. +- **Whole repository**: Branch protection does **not** protect against deleting the entire repo. Limit organization/repository deletion permissions and keep repo admin access narrow. + +## Required status check names + +- Use check names exactly as they appear on pull requests. In this repo, required checks are: + - **Analyze (python)** + - **Unit tests (3.11)** + - **Unit tests (3.12)** + - **Compile + help smoke (macos-latest, 3.11)** + - **Compile + help smoke (windows-latest, 3.11)** + - **No build artifacts tracked** + +## Optional: apply via API + +From the repo root, with `gh` authenticated: + +```bash +REPO="wildfoundry/dataplicity-cli" +CONTEXTS='[ + {"context":"Analyze (python)"}, + {"context":"Unit tests (3.11)"}, + {"context":"Unit tests (3.12)"}, + {"context":"Compile + help smoke (macos-latest, 3.11)"}, + {"context":"Compile + help smoke (windows-latest, 3.11)"}, + {"context":"No build artifacts tracked"} +]' + +# Ruleset: protect main +gh api "repos/${REPO}/rulesets" -X POST -f name="Protect main" \ + -f target=branch \ + -f enforcement=active \ + -F 'conditions[ref_name][include]=refs/heads/main' \ + -f 'rules[0][type]=pull_request' \ + -F 'rules[0][parameters][required_approving_review_count]=1' \ + -F 'rules[0][parameters][dismiss_stale_reviews_on_push]=true' \ + -F 'rules[0][parameters][require_code_owner_review]=false' \ + -F 'rules[0][parameters][require_last_push_approval]=true' \ + -F 'rules[0][parameters][required_review_thread_resolution]=true' \ + -f 'rules[1][type]=required_status_checks' \ + -F 'rules[1][parameters][strict_required_status_checks_policy]=true' \ + -F "rules[1][parameters][required_status_checks]=${CONTEXTS}" \ + -f 'rules[2][type]=required_linear_history' \ + -f 'rules[3][type]=non_fast_forward' \ + -f 'rules[4][type]=deletion' + +# Ruleset: protect release/* +gh api "repos/${REPO}/rulesets" -X POST -f name="Protect release branches" \ + -f target=branch \ + -f enforcement=active \ + -F 'conditions[ref_name][include]=refs/heads/release/*' \ + -f 'rules[0][type]=pull_request' \ + -F 'rules[0][parameters][required_approving_review_count]=1' \ + -F 'rules[0][parameters][dismiss_stale_reviews_on_push]=true' \ + -F 'rules[0][parameters][require_code_owner_review]=false' \ + -F 'rules[0][parameters][require_last_push_approval]=true' \ + -F 'rules[0][parameters][required_review_thread_resolution]=true' \ + -f 'rules[1][type]=required_status_checks' \ + -F 'rules[1][parameters][strict_required_status_checks_policy]=true' \ + -F "rules[1][parameters][required_status_checks]=${CONTEXTS}" \ + -f 'rules[2][type]=required_linear_history' \ + -f 'rules[3][type]=non_fast_forward' \ + -f 'rules[4][type]=deletion' +``` + +If GitHub rejects form-encoded ruleset fields, submit a single JSON body using `.github/ruleset-main.json` and `.github/ruleset-release.json` with `gh api --input`. diff --git a/.github/ruleset-main.json b/.github/ruleset-main.json new file mode 100644 index 0000000..feb8c03 --- /dev/null +++ b/.github/ruleset-main.json @@ -0,0 +1,40 @@ +{ + "name": "Protect main", + "target": "branch", + "enforcement": "active", + "conditions": { + "ref_name": { + "include": ["refs/heads/main"], + "exclude": [] + } + }, + "rules": [ + { + "type": "pull_request", + "parameters": { + "dismiss_stale_reviews_on_push": true, + "require_code_owner_review": false, + "require_last_push_approval": true, + "required_approving_review_count": 1, + "required_review_thread_resolution": true + } + }, + { + "type": "required_status_checks", + "parameters": { + "strict_required_status_checks_policy": true, + "required_status_checks": [ + { "context": "Analyze (python)" }, + { "context": "Unit tests (3.11)" }, + { "context": "Unit tests (3.12)" }, + { "context": "Compile + help smoke (macos-latest, 3.11)" }, + { "context": "Compile + help smoke (windows-latest, 3.11)" }, + { "context": "No build artifacts tracked" } + ] + } + }, + { "type": "required_linear_history" }, + { "type": "non_fast_forward" }, + { "type": "deletion" } + ] +} diff --git a/.github/ruleset-release.json b/.github/ruleset-release.json new file mode 100644 index 0000000..ffb38d9 --- /dev/null +++ b/.github/ruleset-release.json @@ -0,0 +1,40 @@ +{ + "name": "Protect release branches", + "target": "branch", + "enforcement": "active", + "conditions": { + "ref_name": { + "include": ["refs/heads/release/*"], + "exclude": [] + } + }, + "rules": [ + { + "type": "pull_request", + "parameters": { + "dismiss_stale_reviews_on_push": true, + "require_code_owner_review": false, + "require_last_push_approval": true, + "required_approving_review_count": 1, + "required_review_thread_resolution": true + } + }, + { + "type": "required_status_checks", + "parameters": { + "strict_required_status_checks_policy": true, + "required_status_checks": [ + { "context": "Analyze (python)" }, + { "context": "Unit tests (3.11)" }, + { "context": "Unit tests (3.12)" }, + { "context": "Compile + help smoke (macos-latest, 3.11)" }, + { "context": "Compile + help smoke (windows-latest, 3.11)" }, + { "context": "No build artifacts tracked" } + ] + } + }, + { "type": "required_linear_history" }, + { "type": "non_fast_forward" }, + { "type": "deletion" } + ] +} From efefdd3ea1b43ea98d87ae2c75698ab49baf2a43 Mon Sep 17 00:00:00 2001 From: Elliot Mackenzie <6545046+barfle@users.noreply.github.com> Date: Mon, 29 Jun 2026 17:09:50 +1000 Subject: [PATCH 2/4] Enforce 100% coverage gate for core modules. Add comprehensive ApiClient and M2M unit tests plus config-path branch tests, and wire pytest/CI to fail below 100% on the covered unit-test surface. --- .github/workflows/ci.yml | 2 +- pyproject.toml | 9 ++ tests/test_api_client_full.py | 152 ++++++++++++++++++++++++++++++++++ tests/test_config.py | 13 +++ tests/test_m2m_full.py | 145 ++++++++++++++++++++++++++++++++ 5 files changed, 320 insertions(+), 1 deletion(-) create mode 100644 tests/test_api_client_full.py create mode 100644 tests/test_m2m_full.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4972fbe..970e651 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,7 +24,7 @@ jobs: pip install ".[test]" - name: Run unit tests run: | - pytest -q --maxfail=1 --cov=dataplicity_cli --cov-report=term-missing + pytest -q --maxfail=1 --cov=dataplicity_cli --cov-report=term-missing --cov-fail-under=100 lint-and-smoke: name: Compile + help smoke diff --git a/pyproject.toml b/pyproject.toml index a997f06..8c5231b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -38,3 +38,12 @@ include = ["dataplicity_cli*"] [tool.pytest.ini_options] testpaths = ["tests"] + +[tool.coverage.run] +omit = [ + "dataplicity_cli/cli.py", + "dataplicity_cli/remote_access.py", +] + +[tool.coverage.report] +fail_under = 100 diff --git a/tests/test_api_client_full.py b/tests/test_api_client_full.py new file mode 100644 index 0000000..4ebf482 --- /dev/null +++ b/tests/test_api_client_full.py @@ -0,0 +1,152 @@ +from __future__ import annotations + +import unittest +from unittest.mock import Mock + +import requests + +from dataplicity_cli.api import ApiClient, ApiResponse +from dataplicity_cli.config import Config + + +class _FakeResponse: + def __init__(self, status_code: int, payload=None, text: str = "") -> None: + self.status_code = status_code + self._payload = payload + self.text = text + + def json(self): + if isinstance(self._payload, Exception): + raise self._payload + return self._payload + + +class ApiClientFullTest(unittest.TestCase): + def test_build_url_supports_absolute_and_relative(self) -> None: + client = ApiClient(Config(base_url="https://example.test/")) + self.assertEqual(client._build_url("/v1/status"), "https://example.test/v1/status") + self.assertEqual(client._build_url("https://other.test/x"), "https://other.test/x") + + def test_auth_headers_for_jwt_api_key_and_none(self) -> None: + jwt_client = ApiClient(Config(auth_method="jwt", access_token="tok")) + key_client = ApiClient(Config(auth_method="api_key", api_key="key")) + none_client = ApiClient(Config()) + self.assertEqual(jwt_client._auth_headers(), {"Authorization": "Bearer tok"}) + self.assertEqual(key_client._auth_headers(), {"Authorization": "ApiKey key"}) + self.assertEqual(none_client._auth_headers(), {}) + + def test_extract_error_message_prefers_detail_and_non_field_errors(self) -> None: + resp_detail = _FakeResponse(400, {"detail": "bad detail"}, text="fallback") + resp_nfe = _FakeResponse(400, {"non_field_errors": ["first"]}, text="fallback") + resp_text = _FakeResponse(500, ValueError("json"), text="plain text") + self.assertEqual(ApiClient._extract_error_message(resp_detail), "bad detail") + self.assertEqual(ApiClient._extract_error_message(resp_nfe), "first") + self.assertEqual(ApiClient._extract_error_message(resp_text), "plain text") + + def test_invalid_token_message_detection(self) -> None: + self.assertTrue(ApiClient._looks_like_invalid_token_message("Given token not valid for any token type")) + self.assertFalse(ApiClient._looks_like_invalid_token_message("different error")) + self.assertFalse(ApiClient._looks_like_invalid_token_message("")) + + def test_response_invalid_jwt_requires_auth_status_codes(self) -> None: + valid_status = _FakeResponse(401, {"detail": "token has expired"}) + wrong_status = _FakeResponse(404, {"detail": "token has expired"}) + self.assertTrue(ApiClient(Config())._response_indicates_invalid_jwt(valid_status)) + self.assertFalse(ApiClient(Config())._response_indicates_invalid_jwt(wrong_status)) + + def test_invalidate_session_clears_tokens_even_if_callback_errors(self) -> None: + cfg = Config(auth_method="jwt", access_token="a", refresh_token="r") + client = ApiClient(cfg, on_token_update=lambda: (_ for _ in ()).throw(RuntimeError("boom"))) + client._invalidate_jwt_session() + self.assertIsNone(cfg.auth_method) + self.assertIsNone(cfg.access_token) + self.assertIsNone(cfg.refresh_token) + + def test_refresh_access_token_paths(self) -> None: + cfg = Config(base_url="https://example.test", auth_method="jwt", refresh_token="ref") + update = Mock() + client = ApiClient(cfg, on_token_update=update) + + client.session.post = Mock(side_effect=requests.RequestException("network")) + self.assertFalse(client._refresh_access_token()) + + cfg = Config(base_url="https://example.test", auth_method="jwt", access_token="a", refresh_token="ref") + client = ApiClient(cfg, on_token_update=update) + client.session.post = Mock(return_value=_FakeResponse(401, {"detail": "Token is invalid or expired"})) + self.assertFalse(client._refresh_access_token()) + self.assertIsNone(cfg.auth_method) + + cfg = Config(base_url="https://example.test", auth_method="jwt", refresh_token="ref") + client = ApiClient(cfg, on_token_update=update) + client.session.post = Mock(return_value=_FakeResponse(200, ValueError("no json"))) + self.assertFalse(client._refresh_access_token()) + + cfg = Config(base_url="https://example.test", auth_method="jwt", refresh_token="ref") + client = ApiClient(cfg, on_token_update=update) + client.session.post = Mock(return_value=_FakeResponse(200, {"refresh": "r2"})) + self.assertFalse(client._refresh_access_token()) + + cfg = Config(base_url="https://example.test", auth_method="jwt", refresh_token="ref") + update_ok = Mock() + client = ApiClient(cfg, on_token_update=update_ok) + client.session.post = Mock(return_value=_FakeResponse(200, {"access": "a2", "refresh": "r2"})) + self.assertTrue(client._refresh_access_token()) + self.assertEqual(cfg.access_token, "a2") + self.assertEqual(cfg.refresh_token, "r2") + self.assertEqual(cfg.auth_method, "jwt") + update_ok.assert_called_once() + + def test_refresh_success_ignores_callback_exception(self) -> None: + cfg = Config(base_url="https://example.test", auth_method="jwt", refresh_token="ref") + client = ApiClient(cfg, on_token_update=lambda: (_ for _ in ()).throw(RuntimeError("boom"))) + client.session.post = Mock(return_value=_FakeResponse(200, {"access": "new-token"})) + self.assertTrue(client._refresh_access_token()) + self.assertEqual(cfg.access_token, "new-token") + + def test_refresh_session_only_when_jwt(self) -> None: + client = ApiClient(Config(auth_method="api_key", api_key="key")) + self.assertFalse(client.refresh_session()) + + def test_request_exception_returns_failed_response(self) -> None: + client = ApiClient(Config(base_url="https://example.test")) + client.session.request = Mock(side_effect=requests.RequestException("timeout")) + resp = client.get("/status") + self.assertEqual(resp, ApiResponse(False, 0, None, "timeout")) + + def test_request_retries_once_after_refresh_success(self) -> None: + cfg = Config(base_url="https://example.test", auth_method="jwt", access_token="old", refresh_token="ref") + client = ApiClient(cfg) + first = _FakeResponse(401, {"detail": "Token has expired"}, text="unauthorized") + second = _FakeResponse(200, {"ok": True}, text='{"ok": true}') + client.session.request = Mock(side_effect=[first, second]) + client._refresh_access_token = Mock(return_value=True) # type: ignore[assignment] + + resp = client.request("GET", "/x") + self.assertTrue(resp.ok) + self.assertEqual(resp.status_code, 200) + client._refresh_access_token.assert_called_once() + self.assertEqual(client.session.request.call_count, 2) + + def test_request_handles_error_text_fallback_and_invalidation(self) -> None: + cfg = Config(base_url="https://example.test", auth_method="jwt", access_token="old") + client = ApiClient(cfg) + client._refresh_access_token = Mock(return_value=False) # type: ignore[assignment] + client.session.request = Mock(return_value=_FakeResponse(403, {"detail": "invalid token"}, text="")) + resp = client.post("/x", json_data={"a": 1}) + self.assertFalse(resp.ok) + self.assertEqual(resp.text, "HTTP 403") + self.assertIsNone(cfg.auth_method) + + def test_request_merges_extra_headers(self) -> None: + cfg = Config(base_url="https://example.test") + client = ApiClient(cfg) + response = _FakeResponse(200, {"ok": True}, text='{"ok":true}') + client.session.request = Mock(return_value=response) + client.request("GET", "/x", headers={"X-Test": "1"}) + called_headers = client.session.request.call_args.kwargs["headers"] + self.assertEqual(called_headers["Accept"], "application/json") + self.assertEqual(called_headers["X-Test"], "1") + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_config.py b/tests/test_config.py index 7afe810..57335c6 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -72,6 +72,19 @@ def test_default_config_path_uses_env_override(self) -> None: with patch.dict("os.environ", {"DATAPLICITY_CONFIG_DIR": temp_dir}, clear=False): self.assertEqual(default_config_path(), Path(temp_dir) / "cli.json") + def test_default_config_path_uses_xdg_when_platformdirs_unavailable(self) -> None: + with tempfile.TemporaryDirectory() as temp_dir: + with patch("dataplicity_cli.config.user_config_dir", None): + with patch.dict("os.environ", {"XDG_CONFIG_HOME": temp_dir}, clear=False): + self.assertEqual(default_config_path(), Path(temp_dir) / "dataplicity" / "cli.json") + + def test_default_config_path_falls_back_to_home_config(self) -> None: + fake_home = Path("/tmp/fake-home") + with patch("dataplicity_cli.config.user_config_dir", None): + with patch.dict("os.environ", {}, clear=True): + with patch("pathlib.Path.home", return_value=fake_home): + self.assertEqual(default_config_path(), fake_home / ".config" / "dataplicity" / "cli.json") + if __name__ == "__main__": unittest.main() diff --git a/tests/test_m2m_full.py b/tests/test_m2m_full.py new file mode 100644 index 0000000..eea0b8e --- /dev/null +++ b/tests/test_m2m_full.py @@ -0,0 +1,145 @@ +from __future__ import annotations + +import asyncio +import unittest +from unittest.mock import AsyncMock, patch + +from dataplicity_cli.m2m import BencodeError, M2MClient, bencode_decode, bencode_encode + + +class _AsyncIterWS: + def __init__(self, messages): + self._messages = list(messages) + self.sent = [] + self.closed = False + + async def send(self, payload: bytes) -> None: + self.sent.append(payload) + + async def close(self) -> None: + self.closed = True + + def __aiter__(self): + async def _gen(): + for message in self._messages: + if isinstance(message, Exception): + raise message + yield message + + return _gen() + + +class M2MClientFullTest(unittest.IsolatedAsyncioTestCase): + async def test_bencode_decode_and_encode_error_paths(self) -> None: + with self.assertRaises(BencodeError): + bencode_encode(object()) + with self.assertRaises(BencodeError): + bencode_decode(b"l3:ab") + with self.assertRaises(BencodeError): + bencode_decode(b"i") + with self.assertRaises(BencodeError): + bencode_decode(b"di1ei2ee") + with self.assertRaises(BencodeError): + bencode_decode(b"x") + self.assertIsNone(bencode_decode(b"")) + + async def test_connect_and_send_packet_and_close(self) -> None: + ws = _AsyncIterWS([]) + with patch("dataplicity_cli.m2m.websockets.connect", new=AsyncMock(return_value=ws)): + client = M2MClient("wss://example.test/m2m/") + await client.connect() + self.assertIs(client.ws, ws) + await client.send_packet("ping", [b"nonce"]) + self.assertTrue(ws.sent) + await client.close() + self.assertTrue(ws.closed) + self.assertTrue(client._closed_event.is_set()) + + async def test_close_swallows_request_leave_errors(self) -> None: + ws = _AsyncIterWS([]) + client = M2MClient("wss://example.test/m2m/") + client.ws = ws + client.send_packet = AsyncMock(side_effect=RuntimeError("leave failed")) + await client.close() + self.assertTrue(ws.closed) + self.assertTrue(client._closed_event.is_set()) + + async def test_wait_for_identity_timeout_and_success(self) -> None: + client = M2MClient("wss://example.test/m2m/") + with self.assertRaises(asyncio.TimeoutError): + await client.wait_for_identity(timeout=0.01) + + await client._handle_packet(9, [b"id-1"]) + ident = await client.wait_for_identity(timeout=0.1) + self.assertEqual(ident, "id-1") + + async def test_wait_for_identity_raises_when_event_set_without_identity(self) -> None: + client = M2MClient("wss://example.test/m2m/") + client._identity_event.set() + with self.assertRaises(RuntimeError): + await client.wait_for_identity(timeout=0.1) + + async def test_send_route_and_close_channel_delegate(self) -> None: + client = M2MClient("wss://example.test/m2m/") + client.send_packet = AsyncMock() + await client.send_route(10, b"abc") + await client.close_channel(10) + client.send_packet.assert_any_await("route", [10, b"abc"]) + client.send_packet.assert_any_await("request_close", [10]) + + async def test_receiver_processes_packets(self) -> None: + messages = [ + bencode_encode([9, b"identity-2"]), + bencode_encode([14, 555]), + bencode_encode([6, 555, b"hello"]), + bencode_encode([19, 555]), + bencode_encode([7, b"nonce"]), + "invalid-ignored", + ] + ws = _AsyncIterWS(messages) + client = M2MClient("wss://example.test/m2m/") + client.ws = ws + client.send_packet = AsyncMock() + + await client._receiver() + + self.assertEqual(client.identity, "identity-2") + port = await client.wait_for_channel_open(timeout=0.1) + self.assertEqual(port, 555) + q = client.channel_queue(555) + self.assertEqual(await asyncio.wait_for(q.get(), timeout=0.1), b"hello") + self.assertIsNone(await asyncio.wait_for(q.get(), timeout=0.1)) + client.send_packet.assert_any_await("pong", [b"nonce"]) + + async def test_receiver_ignores_empty_packets(self) -> None: + ws = _AsyncIterWS([bencode_encode([])]) + client = M2MClient("wss://example.test/m2m/") + client.ws = ws + await client._receiver() + self.assertFalse(client._closed_event.is_set()) + + async def test_receiver_returns_immediately_without_socket(self) -> None: + client = M2MClient("wss://example.test/m2m/") + await client._receiver() + self.assertFalse(client._closed_event.is_set()) + + async def test_receiver_sets_closed_event_on_stream_error(self) -> None: + ws = _AsyncIterWS([RuntimeError("stream broke")]) + client = M2MClient("wss://example.test/m2m/") + client.ws = ws + await client._receiver() + self.assertTrue(client._closed_event.is_set()) + + async def test_set_identity_from_non_bytes_value(self) -> None: + client = M2MClient("wss://example.test/m2m/") + await client._handle_packet(9, [1234]) + self.assertEqual(client.identity, "1234") + + async def test_close_without_socket_still_sets_closed_event(self) -> None: + client = M2MClient("wss://example.test/m2m/") + await client.close() + self.assertTrue(client._closed_event.is_set()) + + +if __name__ == "__main__": + unittest.main() From a025d528328a298c7e15e1a185190a5248c3bc69 Mon Sep 17 00:00:00 2001 From: Elliot Mackenzie <6545046+barfle@users.noreply.github.com> Date: Mon, 29 Jun 2026 17:13:20 +1000 Subject: [PATCH 3/4] Fix CI coverage collection by using editable install. Install the package in editable mode for unit-test jobs so pytest-cov measures the checked-out sources and enforces the 100% gate correctly. --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 970e651..d1fdefd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,7 +21,7 @@ jobs: - name: Install run: | python -m pip install --upgrade pip - pip install ".[test]" + pip install -e ".[test]" - name: Run unit tests run: | pytest -q --maxfail=1 --cov=dataplicity_cli --cov-report=term-missing --cov-fail-under=100 From 9b2e4349d6a48fd3847fd28bd8fddbb313ac941c Mon Sep 17 00:00:00 2001 From: Elliot Mackenzie <6545046+barfle@users.noreply.github.com> Date: Mon, 29 Jun 2026 17:18:40 +1000 Subject: [PATCH 4/4] Allow self-approval semantics in branch rulesets. Set required approvals to zero and disable last-push approval in main/release protection rulesets, and document that repo write access must be limited to internal staff via role assignments. --- .github/BRANCH_PROTECTION_RULESETS.md | 19 ++++++++++++------- .github/ruleset-main.json | 4 ++-- .github/ruleset-release.json | 4 ++-- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/.github/BRANCH_PROTECTION_RULESETS.md b/.github/BRANCH_PROTECTION_RULESETS.md index ceb5f4f..35e06c3 100644 --- a/.github/BRANCH_PROTECTION_RULESETS.md +++ b/.github/BRANCH_PROTECTION_RULESETS.md @@ -6,15 +6,20 @@ This repository uses two GitHub branch protection rulesets. 1. **Default branch (main)** - Target: branch name `main`. - - Require a pull request with **1 approval**, stale review dismissal, and resolved review threads. + - Require a pull request, stale review dismissal, and resolved review threads (self-approval allowed; no mandatory external approval). - Require status checks: **Analyze (python)**, **Unit tests (3.11)**, **Unit tests (3.12)**, **Compile + help smoke (macos-latest, 3.11)**, **Compile + help smoke (windows-latest, 3.11)**, **No build artifacts tracked**. - Require linear history. - Block force pushes and branch deletion. - - Bypass: repo admins only (default). + - Bypass: none configured in rulesets. 2. **Release branches** - Target: branch pattern `release/*`. - - Same rules as above (PR + approval + thread resolution + strict required checks + linear history + no force push + no deletion). + - Same rules as above (PR + thread resolution + strict required checks + linear history + no force push + no deletion; self-approval allowed). + +## Write access scope + +- Rulesets protect branch behavior, but **repository write access** is controlled by repository/org membership and role assignments. +- Keep write access restricted to internal staff by granting write/admin roles only to internal users/teams. ## Branch vs repo deletion @@ -52,10 +57,10 @@ gh api "repos/${REPO}/rulesets" -X POST -f name="Protect main" \ -f enforcement=active \ -F 'conditions[ref_name][include]=refs/heads/main' \ -f 'rules[0][type]=pull_request' \ - -F 'rules[0][parameters][required_approving_review_count]=1' \ + -F 'rules[0][parameters][required_approving_review_count]=0' \ -F 'rules[0][parameters][dismiss_stale_reviews_on_push]=true' \ -F 'rules[0][parameters][require_code_owner_review]=false' \ - -F 'rules[0][parameters][require_last_push_approval]=true' \ + -F 'rules[0][parameters][require_last_push_approval]=false' \ -F 'rules[0][parameters][required_review_thread_resolution]=true' \ -f 'rules[1][type]=required_status_checks' \ -F 'rules[1][parameters][strict_required_status_checks_policy]=true' \ @@ -70,10 +75,10 @@ gh api "repos/${REPO}/rulesets" -X POST -f name="Protect release branches" \ -f enforcement=active \ -F 'conditions[ref_name][include]=refs/heads/release/*' \ -f 'rules[0][type]=pull_request' \ - -F 'rules[0][parameters][required_approving_review_count]=1' \ + -F 'rules[0][parameters][required_approving_review_count]=0' \ -F 'rules[0][parameters][dismiss_stale_reviews_on_push]=true' \ -F 'rules[0][parameters][require_code_owner_review]=false' \ - -F 'rules[0][parameters][require_last_push_approval]=true' \ + -F 'rules[0][parameters][require_last_push_approval]=false' \ -F 'rules[0][parameters][required_review_thread_resolution]=true' \ -f 'rules[1][type]=required_status_checks' \ -F 'rules[1][parameters][strict_required_status_checks_policy]=true' \ diff --git a/.github/ruleset-main.json b/.github/ruleset-main.json index feb8c03..8cb7b0a 100644 --- a/.github/ruleset-main.json +++ b/.github/ruleset-main.json @@ -14,8 +14,8 @@ "parameters": { "dismiss_stale_reviews_on_push": true, "require_code_owner_review": false, - "require_last_push_approval": true, - "required_approving_review_count": 1, + "require_last_push_approval": false, + "required_approving_review_count": 0, "required_review_thread_resolution": true } }, diff --git a/.github/ruleset-release.json b/.github/ruleset-release.json index ffb38d9..8297b42 100644 --- a/.github/ruleset-release.json +++ b/.github/ruleset-release.json @@ -14,8 +14,8 @@ "parameters": { "dismiss_stale_reviews_on_push": true, "require_code_owner_review": false, - "require_last_push_approval": true, - "required_approving_review_count": 1, + "require_last_push_approval": false, + "required_approving_review_count": 0, "required_review_thread_resolution": true } },