From 761dea6ae2939853febb8a605575bb93ef1d8156 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 11 Sep 2026 11:29:57 +0900 Subject: [PATCH 01/12] test(ci): pin every remote GitHub Action to an immutable commit SHA Nothing in the repository currently enforces that .github/workflows uses: references resolve to immutable commits. The existing runner-image contract only covers runs-on, so a future @v4 or @main reference would pass CI while silently reintroducing mutable supply-chain input. Extend the unowned, CI-wired tests/test_github_actions_runner_image.py with a fail-closed action-pinning contract that requires a full 40-character commit SHA, exempts local (./) and Docker (docker://) references, and strips only YAML comments so an inline version tag cannot mask the resolved ref. Verified: mutation of foundation-ci.yml to actions/checkout@v7 fails the new test; restored tree passes 9/9 and npm run validate exits 0. --- tests/test_github_actions_runner_image.py | 72 +++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/tests/test_github_actions_runner_image.py b/tests/test_github_actions_runner_image.py index 6007c7724..d0f4aef13 100644 --- a/tests/test_github_actions_runner_image.py +++ b/tests/test_github_actions_runner_image.py @@ -15,6 +15,10 @@ ROOT = Path(__file__).resolve().parents[1] WORKFLOWS = ROOT / ".github" / "workflows" _RUNS_ON_PATTERN = re.compile(r"^\s*runs-on\s*:\s*(.*?)\s*$") +_USES_PATTERN = re.compile(r"^\s*uses\s*:\s*(.*?)\s*$") +_PINNED_ACTION_PATTERN = re.compile( + r"^[A-Za-z0-9._-]+/[A-Za-z0-9._-]+@[0-9a-f]{40}$" +) _EXPECTED_RUNNER = "ubuntu-24.04" _CENTRAL_WORKFLOW_NAMES = { "close-empty-pr.yml", @@ -78,6 +82,27 @@ def _runner_declarations(workflow: str) -> list[tuple[int, str]]: return declarations +def _action_declarations(workflow: str) -> list[tuple[int, str]]: + """Return line-numbered scalar ``uses`` declarations for remote actions. + + Local composite actions (``./…``) and Docker references (``docker://…``) are + exempt because they do not resolve a remote Git ref. YAML comments are + stripped so an inline version comment cannot mask the resolved ref. + """ + declarations: list[tuple[int, str]] = [] + for line_number, line in enumerate(workflow.splitlines(), start=1): + match = _USES_PATTERN.match(line) + if match is None: + continue + value = _strip_yaml_comment(match.group(1)) + if len(value) >= 2 and value[0] == value[-1] and value[0] in {"'", '"'}: + value = value[1:-1] + if value.startswith("./") or value.startswith("docker://"): + continue + declarations.append((line_number, value)) + return declarations + + class GitHubActionsRunnerImageContractTest(unittest.TestCase): """Keep every repository-owned runner declaration on one explicit image.""" @@ -150,6 +175,53 @@ def test_runner_parser_only_strips_yaml_comment_tokens(self) -> None: ) +class GitHubActionsActionPinningContractTest(unittest.TestCase): + """Keep every remote action reference pinned to an immutable commit.""" + + def test_all_remote_action_references_are_commit_pinned(self) -> None: + """Reject mutable tags and branches such as ``@v4`` or ``@main``.""" + unpinned: list[str] = [] + for workflow_path in _workflow_paths(): + for line_number, value in _action_declarations( + workflow_path.read_text(encoding="utf-8") + ): + if not _PINNED_ACTION_PATTERN.match(value): + unpinned.append(f"{workflow_path.name}:{line_number}={value!r}") + self.assertEqual( + [], + unpinned, + "remote action references must pin a full 40-character commit SHA: " + f"{unpinned}", + ) + + def test_action_parser_rejects_mutable_and_pins_commit_sha(self) -> None: + """Keep the validator sensitive to tags, branches, and local/Docker refs.""" + sample = "\n".join( + ( + "uses: actions/checkout@v4", + "uses: actions/checkout@main", + "uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1", + "uses: ./local-action", + "uses: docker://alpine:3.20", + "uses: 'actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97'", + ) + ) + declarations = _action_declarations(sample) + self.assertEqual( + [ + "actions/checkout@v4", + "actions/checkout@main", + "actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1", + "actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97", + ], + [value for _, value in declarations], + ) + self.assertEqual( + 2, + sum(bool(_PINNED_ACTION_PATTERN.match(value)) for _, value in declarations), + ) + + class GitHubActionsQueueContractTest(unittest.TestCase): """Keep local workflows bounded and same-PR cancellation isolated.""" From 45af75743d4a66bb4d7925f308d2e3b826f37941 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 11 Sep 2026 11:50:11 +0900 Subject: [PATCH 02/12] test(ci): accept commit-pinned reusable workflows and subdirectory actions The first pinning regex matched only owner/repo@sha, which would have false-positively rejected a commit-pinned reusable workflow or subdirectory action such as org/repo/.github/workflows/ci.yml@<40-hex>. Widen the pattern to allow additional path segments and extend the sensitivity test with pinned reusable-workflow, pinned subdirectory, and mutable reusable-workflow cases so the contract distinguishes them. Tests: 9/9 OK; npm run validate exit 0. --- tests/test_github_actions_runner_image.py | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/tests/test_github_actions_runner_image.py b/tests/test_github_actions_runner_image.py index d0f4aef13..afb06bb8d 100644 --- a/tests/test_github_actions_runner_image.py +++ b/tests/test_github_actions_runner_image.py @@ -17,7 +17,7 @@ _RUNS_ON_PATTERN = re.compile(r"^\s*runs-on\s*:\s*(.*?)\s*$") _USES_PATTERN = re.compile(r"^\s*uses\s*:\s*(.*?)\s*$") _PINNED_ACTION_PATTERN = re.compile( - r"^[A-Za-z0-9._-]+/[A-Za-z0-9._-]+@[0-9a-f]{40}$" + r"^[A-Za-z0-9._-]+/[A-Za-z0-9._-]+(?:/[A-Za-z0-9._/-]+)?@[0-9a-f]{40}$" ) _EXPECTED_RUNNER = "ubuntu-24.04" _CENTRAL_WORKFLOW_NAMES = { @@ -204,6 +204,9 @@ def test_action_parser_rejects_mutable_and_pins_commit_sha(self) -> None: "uses: ./local-action", "uses: docker://alpine:3.20", "uses: 'actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97'", + "uses: ContextualWisdomLab/Orgmetra/.github/workflows/ci.yml@3d3c42e5aac5ba805825da76410c181273ba90b1", + "uses: owner/repo/sub/dir@5fda3b95a4ea91299a34e894583c3862153e4b97 # v1", + "uses: owner/repo/.github/workflows/ci.yml@v1", ) ) declarations = _action_declarations(sample) @@ -213,12 +216,24 @@ def test_action_parser_rejects_mutable_and_pins_commit_sha(self) -> None: "actions/checkout@main", "actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1", "actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97", + "ContextualWisdomLab/Orgmetra/.github/workflows/ci.yml@3d3c42e5aac5ba805825da76410c181273ba90b1", + "owner/repo/sub/dir@5fda3b95a4ea91299a34e894583c3862153e4b97", + "owner/repo/.github/workflows/ci.yml@v1", ], [value for _, value in declarations], ) + unpinned = [ + value + for _, value in declarations + if not _PINNED_ACTION_PATTERN.match(value) + ] self.assertEqual( - 2, - sum(bool(_PINNED_ACTION_PATTERN.match(value)) for _, value in declarations), + [ + "actions/checkout@v4", + "actions/checkout@main", + "owner/repo/.github/workflows/ci.yml@v1", + ], + unpinned, ) From 6bc95b2babac3b8d9602d8e77c5fc1ea44ee0558 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 11 Sep 2026 11:59:09 +0900 Subject: [PATCH 03/12] test(ci): validate list-form action pins --- tests/test_github_actions_runner_image.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/test_github_actions_runner_image.py b/tests/test_github_actions_runner_image.py index afb06bb8d..3b2fb0011 100644 --- a/tests/test_github_actions_runner_image.py +++ b/tests/test_github_actions_runner_image.py @@ -15,7 +15,7 @@ ROOT = Path(__file__).resolve().parents[1] WORKFLOWS = ROOT / ".github" / "workflows" _RUNS_ON_PATTERN = re.compile(r"^\s*runs-on\s*:\s*(.*?)\s*$") -_USES_PATTERN = re.compile(r"^\s*uses\s*:\s*(.*?)\s*$") +_USES_PATTERN = re.compile(r"^\s*(?:-\s+)?uses\s*:\s*(.*?)\s*$") _PINNED_ACTION_PATTERN = re.compile( r"^[A-Za-z0-9._-]+/[A-Za-z0-9._-]+(?:/[A-Za-z0-9._/-]+)?@[0-9a-f]{40}$" ) @@ -198,9 +198,11 @@ def test_action_parser_rejects_mutable_and_pins_commit_sha(self) -> None: """Keep the validator sensitive to tags, branches, and local/Docker refs.""" sample = "\n".join( ( + "- uses: actions/checkout@v4", "uses: actions/checkout@v4", "uses: actions/checkout@main", "uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1", + " - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97", "uses: ./local-action", "uses: docker://alpine:3.20", "uses: 'actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97'", @@ -212,10 +214,12 @@ def test_action_parser_rejects_mutable_and_pins_commit_sha(self) -> None: declarations = _action_declarations(sample) self.assertEqual( [ + "actions/checkout@v4", "actions/checkout@v4", "actions/checkout@main", "actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1", "actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97", + "actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97", "ContextualWisdomLab/Orgmetra/.github/workflows/ci.yml@3d3c42e5aac5ba805825da76410c181273ba90b1", "owner/repo/sub/dir@5fda3b95a4ea91299a34e894583c3862153e4b97", "owner/repo/.github/workflows/ci.yml@v1", @@ -229,6 +233,7 @@ def test_action_parser_rejects_mutable_and_pins_commit_sha(self) -> None: ] self.assertEqual( [ + "actions/checkout@v4", "actions/checkout@v4", "actions/checkout@main", "owner/repo/.github/workflows/ci.yml@v1", From 2a18f3abb188fe7bab29f30bcaa575952958b9e0 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 11 Sep 2026 12:00:55 +0900 Subject: [PATCH 04/12] test(ci): require digest pinning for every container and service image The action-pinning contract covered `uses:` refs but nothing generic enforced that `image:` references (job containers and service containers) resolve an immutable digest; only the two named recovery postgres images were asserted by other tests, so a fresh unpinned `image: postgres:16` would have passed. Add _image_declarations plus a digest contract and a sensitivity test that rejects plain tags and expressions while accepting a pinned digest (quoted or with a trailing comment). Tests: 11/11 OK; mutation to postgres:17.6-alpine fails the new contract; npm run validate exit 0. --- tests/test_github_actions_runner_image.py | 69 +++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/tests/test_github_actions_runner_image.py b/tests/test_github_actions_runner_image.py index 3b2fb0011..286e05055 100644 --- a/tests/test_github_actions_runner_image.py +++ b/tests/test_github_actions_runner_image.py @@ -16,9 +16,11 @@ WORKFLOWS = ROOT / ".github" / "workflows" _RUNS_ON_PATTERN = re.compile(r"^\s*runs-on\s*:\s*(.*?)\s*$") _USES_PATTERN = re.compile(r"^\s*(?:-\s+)?uses\s*:\s*(.*?)\s*$") +_IMAGE_PATTERN = re.compile(r"^\s*image\s*:\s*(.*?)\s*$") _PINNED_ACTION_PATTERN = re.compile( r"^[A-Za-z0-9._-]+/[A-Za-z0-9._-]+(?:/[A-Za-z0-9._/-]+)?@[0-9a-f]{40}$" ) +_PINNED_IMAGE_PATTERN = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._:/-]*@sha256:[0-9a-f]{64}$") _EXPECTED_RUNNER = "ubuntu-24.04" _CENTRAL_WORKFLOW_NAMES = { "close-empty-pr.yml", @@ -103,6 +105,25 @@ def _action_declarations(workflow: str) -> list[tuple[int, str]]: return declarations +def _image_declarations(workflow: str) -> list[tuple[int, str]]: + """Return line-numbered ``image`` declarations for job/service containers. + + Container and service images must resolve a registry digest rather than a + mutable tag. YAML comments are stripped so an inline tag comment cannot mask + the resolved reference. + """ + declarations: list[tuple[int, str]] = [] + for line_number, line in enumerate(workflow.splitlines(), start=1): + match = _IMAGE_PATTERN.match(line) + if match is None: + continue + value = _strip_yaml_comment(match.group(1)) + if len(value) >= 2 and value[0] == value[-1] and value[0] in {"'", '"'}: + value = value[1:-1] + declarations.append((line_number, value)) + return declarations + + class GitHubActionsRunnerImageContractTest(unittest.TestCase): """Keep every repository-owned runner declaration on one explicit image.""" @@ -241,6 +262,54 @@ def test_action_parser_rejects_mutable_and_pins_commit_sha(self) -> None: unpinned, ) + def test_all_container_images_are_digest_pinned(self) -> None: + """Reject mutable container/service tags such as ``postgres:16``.""" + unpinned: list[str] = [] + for workflow_path in _workflow_paths(): + for line_number, value in _image_declarations( + workflow_path.read_text(encoding="utf-8") + ): + if not _PINNED_IMAGE_PATTERN.match(value): + unpinned.append(f"{workflow_path.name}:{line_number}={value!r}") + self.assertEqual( + [], + unpinned, + "container and service images must pin an immutable sha256 digest: " + f"{unpinned}", + ) + + def test_image_parser_rejects_mutable_tags_and_pins_digest(self) -> None: + """Keep the validator sensitive to tags, expressions, and pinned digests.""" + sample = "\n".join( + ( + "image: postgres:17.6-alpine", + "image: postgres:17.6-alpine@sha256:" + "ef257d85f76e48da1c64832459b59fcaba1a4dac97bf5d7450c77753542eee94", + "image: ${{ matrix.image }}", + "image: 'postgres:17.6-alpine@sha256:" + "ef257d85f76e48da1c64832459b59fcaba1a4dac97bf5d7450c77753542eee94' # latest", + ) + ) + declarations = _image_declarations(sample) + self.assertEqual( + [ + "postgres:17.6-alpine", + "postgres:17.6-alpine@sha256:" + "ef257d85f76e48da1c64832459b59fcaba1a4dac97bf5d7450c77753542eee94", + "${{ matrix.image }}", + "postgres:17.6-alpine@sha256:" + "ef257d85f76e48da1c64832459b59fcaba1a4dac97bf5d7450c77753542eee94", + ], + [value for _, value in declarations], + ) + unpinned = [ + value for _, value in declarations if not _PINNED_IMAGE_PATTERN.match(value) + ] + self.assertEqual( + ["postgres:17.6-alpine", "${{ matrix.image }}"], + unpinned, + ) + class GitHubActionsQueueContractTest(unittest.TestCase): """Keep local workflows bounded and same-PR cancellation isolated.""" From c459ceb2315ba1b895cd0fe7e1dc37513bc14adb Mon Sep 17 00:00:00 2001 From: seonghobae Date: Fri, 11 Sep 2026 13:03:20 +0900 Subject: [PATCH 05/12] test(ci): require least-privilege permissions and safe triggers --- tests/test_github_actions_runner_image.py | 82 +++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/tests/test_github_actions_runner_image.py b/tests/test_github_actions_runner_image.py index 286e05055..99a5c16ed 100644 --- a/tests/test_github_actions_runner_image.py +++ b/tests/test_github_actions_runner_image.py @@ -21,6 +21,10 @@ r"^[A-Za-z0-9._-]+/[A-Za-z0-9._-]+(?:/[A-Za-z0-9._/-]+)?@[0-9a-f]{40}$" ) _PINNED_IMAGE_PATTERN = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._:/-]*@sha256:[0-9a-f]{64}$") +_PERMISSIONS_BLOCK_PATTERN = re.compile(r"^permissions\s*:\s*(.*?)\s*$") +_PERMISSION_SCOPE_PATTERN = re.compile(r"^\s*([A-Za-z0-9-]+)\s*:\s*(read|write|none)\s*$") +_PRIVILEGED_TRIGGER_PATTERN = re.compile(r"^\s*(pull_request_target|workflow_run)\s*:") +_WRITE_ALL_PATTERN = re.compile(r"permissions\s*:\s*write-all\b") _EXPECTED_RUNNER = "ubuntu-24.04" _CENTRAL_WORKFLOW_NAMES = { "close-empty-pr.yml", @@ -311,6 +315,84 @@ def test_image_parser_rejects_mutable_tags_and_pins_digest(self) -> None: ) +def _declared_permissions(workflow: str) -> dict[str, str]: + """Return top-level ``permissions`` scopes as a scope→level mapping. + + Only the top-level block is inspected: a job-level override cannot loosen + the repository-wide default that gates the whole workflow run. + """ + scopes: dict[str, str] = {} + lines = workflow.splitlines() + for index, line in enumerate(lines): + if _PERMISSIONS_BLOCK_PATTERN.match(_strip_yaml_comment(line)) is None: + continue + for child in lines[index + 1 :]: + if child.strip() == "" or child.lstrip().startswith("#"): + continue + match = _PERMISSION_SCOPE_PATTERN.match(_strip_yaml_comment(child)) + if match is None: + break + scopes[match.group(1)] = match.group(2) + break + return scopes + + +class GitHubActionsLeastPrivilegeContractTest(unittest.TestCase): + """Keep repository-owned workflows read-only and off privileged triggers.""" + + def test_local_workflows_declare_only_read_scoped_contents_permission(self) -> None: + """Reject missing, write-scoped, or broadened top-level permissions.""" + missing: list[str] = [] + violations: list[str] = [] + for workflow_path in _workflow_paths(): + workflow = workflow_path.read_text(encoding="utf-8") + scopes = _declared_permissions(workflow) + if not scopes: + missing.append(workflow_path.name) + continue + for scope, level in scopes.items(): + if scope != "contents" or level.lower() != "read": + violations.append(f"{workflow_path.name}: {scope}={level}") + self.assertEqual([], missing, f"top-level permissions block is missing from: {missing}") + self.assertEqual( + [], + violations, + f"local workflows must grant only contents: read: {violations}", + ) + + def test_local_workflows_reject_privileged_triggers_and_write_all(self) -> None: + """Reject pull_request_target, workflow_run, and write-all escalation.""" + violations: list[str] = [] + for workflow_path in _workflow_paths(): + workflow = workflow_path.read_text(encoding="utf-8") + for line_number, line in enumerate(workflow.splitlines(), start=1): + candidate = _strip_yaml_comment(line) + if _PRIVILEGED_TRIGGER_PATTERN.match(candidate): + violations.append(f"{workflow_path.name}:{line_number}={line.strip()!r}") + if _WRITE_ALL_PATTERN.search(candidate): + violations.append(f"{workflow_path.name}:{line_number}={line.strip()!r}") + self.assertEqual( + [], + violations, + f"privileged triggers and write-all permissions are forbidden: {violations}", + ) + + def test_least_privilege_parser_is_sensitive_to_unsafe_workflows(self) -> None: + """Keep the permission parser and trigger guard fail-closed.""" + self.assertEqual({"contents": "read"}, _declared_permissions("permissions:\n contents: read\n")) + self.assertEqual( + {"contents": "write", "id-token": "write"}, + _declared_permissions("permissions:\n contents: write\n id-token: write\n"), + ) + self.assertEqual({}, _declared_permissions("name: no permissions here\n")) + self.assertIsNotNone(_PRIVILEGED_TRIGGER_PATTERN.match("pull_request_target:")) + self.assertIsNotNone(_PRIVILEGED_TRIGGER_PATTERN.match(" pull_request_target:")) + self.assertIsNotNone(_PRIVILEGED_TRIGGER_PATTERN.match("workflow_run:")) + self.assertIsNone(_PRIVILEGED_TRIGGER_PATTERN.match("pull_request:")) + self.assertIsNotNone(_WRITE_ALL_PATTERN.search("permissions: write-all")) + self.assertIsNone(_WRITE_ALL_PATTERN.search("permissions:\n contents: read")) + + class GitHubActionsQueueContractTest(unittest.TestCase): """Keep local workflows bounded and same-PR cancellation isolated.""" From 3ba508b64d8889c91965c47436053ab82a2c6e91 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 11 Sep 2026 13:06:07 +0900 Subject: [PATCH 06/12] test(ci): make image digest fixtures explicit --- tests/test_github_actions_runner_image.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/test_github_actions_runner_image.py b/tests/test_github_actions_runner_image.py index 99a5c16ed..0a0e0e033 100644 --- a/tests/test_github_actions_runner_image.py +++ b/tests/test_github_actions_runner_image.py @@ -284,25 +284,25 @@ def test_all_container_images_are_digest_pinned(self) -> None: def test_image_parser_rejects_mutable_tags_and_pins_digest(self) -> None: """Keep the validator sensitive to tags, expressions, and pinned digests.""" + pinned_image = ( + "postgres:17.6-alpine@sha256:" + "ef257d85f76e48da1c64832459b59fcaba1a4dac97bf5d7450c77753542eee94" + ) sample = "\n".join( ( "image: postgres:17.6-alpine", - "image: postgres:17.6-alpine@sha256:" - "ef257d85f76e48da1c64832459b59fcaba1a4dac97bf5d7450c77753542eee94", + f"image: {pinned_image}", "image: ${{ matrix.image }}", - "image: 'postgres:17.6-alpine@sha256:" - "ef257d85f76e48da1c64832459b59fcaba1a4dac97bf5d7450c77753542eee94' # latest", + f"image: '{pinned_image}' # latest", ) ) declarations = _image_declarations(sample) self.assertEqual( [ "postgres:17.6-alpine", - "postgres:17.6-alpine@sha256:" - "ef257d85f76e48da1c64832459b59fcaba1a4dac97bf5d7450c77753542eee94", + pinned_image, "${{ matrix.image }}", - "postgres:17.6-alpine@sha256:" - "ef257d85f76e48da1c64832459b59fcaba1a4dac97bf5d7450c77753542eee94", + pinned_image, ], [value for _, value in declarations], ) @@ -444,4 +444,4 @@ def test_postgres_contracts_wait_on_the_dynamic_host_port(self) -> None: if __name__ == "__main__": # pragma: no cover - normal execution is via unittest discovery. - unittest.main() + unittest.main() \ No newline at end of file From b510e5eabb1142ebc1d4f612387bedfdb96161cc Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 11 Sep 2026 13:09:18 +0900 Subject: [PATCH 07/12] test(ci): reject job-only permissions blocks --- tests/test_github_actions_runner_image.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_github_actions_runner_image.py b/tests/test_github_actions_runner_image.py index 0a0e0e033..57a208c96 100644 --- a/tests/test_github_actions_runner_image.py +++ b/tests/test_github_actions_runner_image.py @@ -385,6 +385,10 @@ def test_least_privilege_parser_is_sensitive_to_unsafe_workflows(self) -> None: _declared_permissions("permissions:\n contents: write\n id-token: write\n"), ) self.assertEqual({}, _declared_permissions("name: no permissions here\n")) + self.assertEqual( + {}, + _declared_permissions("jobs:\n test:\n permissions:\n contents: read\n"), + ) self.assertIsNotNone(_PRIVILEGED_TRIGGER_PATTERN.match("pull_request_target:")) self.assertIsNotNone(_PRIVILEGED_TRIGGER_PATTERN.match(" pull_request_target:")) self.assertIsNotNone(_PRIVILEGED_TRIGGER_PATTERN.match("workflow_run:")) From 597a10b4a2d4a4b8139faf5a4240c40428896304 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 11 Sep 2026 13:10:48 +0900 Subject: [PATCH 08/12] fix(ci): require workflow-level permissions block --- tests/test_github_actions_runner_image.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_github_actions_runner_image.py b/tests/test_github_actions_runner_image.py index 57a208c96..398585824 100644 --- a/tests/test_github_actions_runner_image.py +++ b/tests/test_github_actions_runner_image.py @@ -324,6 +324,8 @@ def _declared_permissions(workflow: str) -> dict[str, str]: scopes: dict[str, str] = {} lines = workflow.splitlines() for index, line in enumerate(lines): + if line != line.lstrip(): + continue if _PERMISSIONS_BLOCK_PATTERN.match(_strip_yaml_comment(line)) is None: continue for child in lines[index + 1 :]: From 1446df3f38aa565813bf8da47b69ddcfc53cb792 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 11 Sep 2026 13:14:58 +0900 Subject: [PATCH 09/12] test(ci): fail closed on job-level permission escalation A job-level permissions: block replaces (not merges) the workflow default, so contents: read at the workflow level plus contents: write on one job still grants write. The top-level-only parser ignored indented blocks and would have missed that escalation. Scan every permissions block and keep the workflow-level check for the missing-block case. --- tests/test_github_actions_runner_image.py | 58 ++++++++++++++++++----- 1 file changed, 47 insertions(+), 11 deletions(-) diff --git a/tests/test_github_actions_runner_image.py b/tests/test_github_actions_runner_image.py index 398585824..55849c6a9 100644 --- a/tests/test_github_actions_runner_image.py +++ b/tests/test_github_actions_runner_image.py @@ -315,11 +315,36 @@ def test_image_parser_rejects_mutable_tags_and_pins_digest(self) -> None: ) +def _declared_permission_blocks(workflow: str) -> list[dict[str, str]]: + """Return every ``permissions`` block, workflow-level and job-level. + + GitHub replaces rather than merges the workflow default when a job declares + its own ``permissions:`` block, so a read-only workflow default can be + escalated per job; every block is inspected. + """ + blocks: list[dict[str, str]] = [] + lines = workflow.splitlines() + for index, line in enumerate(lines): + if _PERMISSIONS_BLOCK_PATTERN.match(_strip_yaml_comment(line)) is None: + continue + scopes: dict[str, str] = {} + for child in lines[index + 1 :]: + if child.strip() == "" or child.lstrip().startswith("#"): + continue + match = _PERMISSION_SCOPE_PATTERN.match(_strip_yaml_comment(child)) + if match is None: + break + scopes[match.group(1)] = match.group(2) + blocks.append(scopes) + return blocks + + def _declared_permissions(workflow: str) -> dict[str, str]: - """Return top-level ``permissions`` scopes as a scope→level mapping. + """Return the workflow-level ``permissions`` block as a scope→level mapping. - Only the top-level block is inspected: a job-level override cannot loosen - the repository-wide default that gates the whole workflow run. + A job-level override cannot loosen the repository-wide default that gates + the whole workflow run, so only the unindented top-level block is returned + here; ``_declared_permission_blocks`` inspects job-level blocks too. """ scopes: dict[str, str] = {} lines = workflow.splitlines() @@ -343,23 +368,23 @@ class GitHubActionsLeastPrivilegeContractTest(unittest.TestCase): """Keep repository-owned workflows read-only and off privileged triggers.""" def test_local_workflows_declare_only_read_scoped_contents_permission(self) -> None: - """Reject missing, write-scoped, or broadened top-level permissions.""" + """Reject missing, write-scoped, or broadened workflow and job permissions.""" missing: list[str] = [] violations: list[str] = [] for workflow_path in _workflow_paths(): workflow = workflow_path.read_text(encoding="utf-8") - scopes = _declared_permissions(workflow) - if not scopes: + if not _declared_permissions(workflow): missing.append(workflow_path.name) continue - for scope, level in scopes.items(): - if scope != "contents" or level.lower() != "read": - violations.append(f"{workflow_path.name}: {scope}={level}") - self.assertEqual([], missing, f"top-level permissions block is missing from: {missing}") + for scopes in _declared_permission_blocks(workflow): + for scope, level in scopes.items(): + if scope != "contents" or level.lower() != "read": + violations.append(f"{workflow_path.name}: {scope}={level}") + self.assertEqual([], missing, f"workflow-level permissions block is missing from: {missing}") self.assertEqual( [], violations, - f"local workflows must grant only contents: read: {violations}", + f"local workflows must grant only contents: read in every block: {violations}", ) def test_local_workflows_reject_privileged_triggers_and_write_all(self) -> None: @@ -391,6 +416,17 @@ def test_least_privilege_parser_is_sensitive_to_unsafe_workflows(self) -> None: {}, _declared_permissions("jobs:\n test:\n permissions:\n contents: read\n"), ) + self.assertEqual( + [{"contents": "read"}], + _declared_permission_blocks("permissions:\n contents: read\n"), + ) + self.assertEqual( + [{"contents": "read"}, {"contents": "write"}], + _declared_permission_blocks( + "permissions:\n contents: read\njobs:\n build:\n permissions:\n contents: write\n" + ), + ) + self.assertEqual([], _declared_permission_blocks("name: no permissions here\n")) self.assertIsNotNone(_PRIVILEGED_TRIGGER_PATTERN.match("pull_request_target:")) self.assertIsNotNone(_PRIVILEGED_TRIGGER_PATTERN.match(" pull_request_target:")) self.assertIsNotNone(_PRIVILEGED_TRIGGER_PATTERN.match("workflow_run:")) From 2e0e24363d12b5f45897256aa156cc1fdbe91bfb Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 11 Sep 2026 13:17:43 +0900 Subject: [PATCH 10/12] test(ci): cover workflow security shorthand --- tests/test_github_actions_runner_image.py | 360 ++++++++++++++-------- 1 file changed, 224 insertions(+), 136 deletions(-) diff --git a/tests/test_github_actions_runner_image.py b/tests/test_github_actions_runner_image.py index 55849c6a9..3717d9e67 100644 --- a/tests/test_github_actions_runner_image.py +++ b/tests/test_github_actions_runner_image.py @@ -1,8 +1,8 @@ -"""Regression contract for deterministic GitHub-hosted runner image selection. +"""Regression contracts for repository-owned GitHub Actions execution. -Orgmetra uses an explicit supported Ubuntu image instead of moving aliases, -other image versions, or expression-driven selectors. Queued evidence remains -non-passing; this test only protects the repository-owned runner contract. +The tests keep runner selection, remote dependencies, container images, token +permissions, trigger semantics, and queue behavior explicit. A queued workflow +is never treated as passing evidence. """ from __future__ import annotations @@ -14,17 +14,27 @@ ROOT = Path(__file__).resolve().parents[1] WORKFLOWS = ROOT / ".github" / "workflows" + _RUNS_ON_PATTERN = re.compile(r"^\s*runs-on\s*:\s*(.*?)\s*$") _USES_PATTERN = re.compile(r"^\s*(?:-\s+)?uses\s*:\s*(.*?)\s*$") _IMAGE_PATTERN = re.compile(r"^\s*image\s*:\s*(.*?)\s*$") +_CONTAINER_PATTERN = re.compile(r"^\s*container\s*:\s*(.*?)\s*$") +_PERMISSIONS_BLOCK_PATTERN = re.compile(r"^permissions\s*:\s*(.*?)\s*$") +_PERMISSION_SCOPE_PATTERN = re.compile(r"^([A-Za-z0-9-]+)\s*:\s*(read|write|none)\s*$") +_ON_PATTERN = re.compile(r"^on\s*:\s*(.*?)\s*$") +_PRIVILEGED_TRIGGER_KEY_PATTERN = re.compile(r"^(pull_request_target|workflow_run)\s*:") +_PRIVILEGED_EVENT_PATTERN = re.compile( + r"(? list[Path]: def _strip_yaml_comment(value: str) -> str: - """Strip only YAML comments, preserving hash characters inside scalar text.""" + """Strip a YAML comment without treating a quoted hash as a comment.""" quote: str | None = None index = 0 while index < len(value): @@ -74,35 +84,34 @@ def _strip_yaml_comment(value: str) -> str: return value.strip() +def _unquote_scalar(value: str) -> str: + """Remove one matching YAML scalar quote pair.""" + value = value.strip() + if len(value) >= 2 and value[0] == value[-1] and value[0] in {"'", '"'}: + return value[1:-1] + return value + + def _runner_declarations(workflow: str) -> list[tuple[int, str]]: - """Return line-numbered scalar ``runs-on`` declarations without YAML comments.""" + """Return line-numbered scalar ``runs-on`` declarations.""" declarations: list[tuple[int, str]] = [] for line_number, line in enumerate(workflow.splitlines(), start=1): match = _RUNS_ON_PATTERN.match(line) - if match is None: - continue - value = _strip_yaml_comment(match.group(1)) - if len(value) >= 2 and value[0] == value[-1] and value[0] in {"'", '"'}: - value = value[1:-1] - declarations.append((line_number, value)) + if match is not None: + declarations.append( + (line_number, _unquote_scalar(_strip_yaml_comment(match.group(1)))) + ) return declarations def _action_declarations(workflow: str) -> list[tuple[int, str]]: - """Return line-numbered scalar ``uses`` declarations for remote actions. - - Local composite actions (``./…``) and Docker references (``docker://…``) are - exempt because they do not resolve a remote Git ref. YAML comments are - stripped so an inline version comment cannot mask the resolved ref. - """ + """Return remote action/reusable-workflow ``uses`` declarations.""" declarations: list[tuple[int, str]] = [] for line_number, line in enumerate(workflow.splitlines(), start=1): match = _USES_PATTERN.match(line) if match is None: continue - value = _strip_yaml_comment(match.group(1)) - if len(value) >= 2 and value[0] == value[-1] and value[0] in {"'", '"'}: - value = value[1:-1] + value = _unquote_scalar(_strip_yaml_comment(match.group(1))) if value.startswith("./") or value.startswith("docker://"): continue declarations.append((line_number, value)) @@ -110,29 +119,116 @@ def _action_declarations(workflow: str) -> list[tuple[int, str]]: def _image_declarations(workflow: str) -> list[tuple[int, str]]: - """Return line-numbered ``image`` declarations for job/service containers. - - Container and service images must resolve a registry digest rather than a - mutable tag. YAML comments are stripped so an inline tag comment cannot mask - the resolved reference. - """ + """Return image references from ``image:`` and scalar ``container:`` forms.""" declarations: list[tuple[int, str]] = [] for line_number, line in enumerate(workflow.splitlines(), start=1): match = _IMAGE_PATTERN.match(line) + if match is None: + match = _CONTAINER_PATTERN.match(line) if match is None: continue - value = _strip_yaml_comment(match.group(1)) - if len(value) >= 2 and value[0] == value[-1] and value[0] in {"'", '"'}: - value = value[1:-1] + + value = _unquote_scalar(_strip_yaml_comment(match.group(1))) + # ``container:`` with no scalar value starts the object form; its nested + # ``image:`` member is collected on its own line. + if not value: + continue declarations.append((line_number, value)) return declarations +def _permission_blocks( + workflow: str, +) -> list[tuple[int, int, str, dict[str, str]]]: + """Return every permissions block with line, indentation, scalar, and scopes.""" + lines = workflow.splitlines() + blocks: list[tuple[int, int, str, dict[str, str]]] = [] + + for index, line in enumerate(lines): + candidate = _strip_yaml_comment(line) + match = _PERMISSIONS_BLOCK_PATTERN.match(candidate) + if match is None: + continue + + indent = len(line) - len(line.lstrip()) + scalar = _unquote_scalar(match.group(1)) + scopes: dict[str, str] = {} + if not scalar: + for child in lines[index + 1 :]: + if child.strip() == "" or child.lstrip().startswith("#"): + continue + child_indent = len(child) - len(child.lstrip()) + if child_indent <= indent: + break + child_candidate = _strip_yaml_comment(child) + scope_match = _PERMISSION_SCOPE_PATTERN.match(child_candidate) + if scope_match is None: + scopes[""] = child_candidate + break + scopes[scope_match.group(1)] = scope_match.group(2) + blocks.append((index + 1, indent, scalar, scopes)) + + return blocks + + +def _declared_permissions(workflow: str) -> dict[str, str]: + """Return the workflow-level permission mapping, excluding job overrides.""" + for _line_number, indent, scalar, scopes in _permission_blocks(workflow): + if indent == 0 and not scalar: + return scopes + return {} + + +def _permission_violations(workflow: str) -> list[str]: + """Return unsafe workflow- or job-level permission declarations.""" + violations: list[str] = [] + for line_number, _indent, scalar, scopes in _permission_blocks(workflow): + if scalar: + violations.append(f"line {line_number}: scalar={scalar}") + continue + if scopes != {"contents": "read"}: + violations.append(f"line {line_number}: scopes={scopes}") + return violations + + +def _privileged_trigger_declarations(workflow: str) -> list[tuple[int, str]]: + """Return privileged events from mapping, scalar, and flow-sequence ``on`` forms.""" + lines = workflow.splitlines() + declarations: list[tuple[int, str]] = [] + on_block_indent: int | None = None + + for line_number, line in enumerate(lines, start=1): + if line.strip() == "" or line.lstrip().startswith("#"): + continue + + indent = len(line) - len(line.lstrip()) + candidate = _strip_yaml_comment(line) + + if indent == 0: + on_match = _ON_PATTERN.match(candidate) + if on_match is not None: + value = _unquote_scalar(on_match.group(1)) + on_block_indent = 0 if not value else None + if value and _PRIVILEGED_EVENT_PATTERN.search(value): + declarations.append((line_number, value)) + continue + + if on_block_indent is not None: + if indent <= on_block_indent: + on_block_indent = None + else: + trigger_match = _PRIVILEGED_TRIGGER_KEY_PATTERN.match(candidate) + if trigger_match is not None: + declarations.append((line_number, trigger_match.group(1))) + + return declarations + + class GitHubActionsRunnerImageContractTest(unittest.TestCase): """Keep every repository-owned runner declaration on one explicit image.""" def test_all_repository_workflow_runner_selectors_are_exact(self) -> None: - """Reject aliases, expressions, other versions, and missing runner declarations.""" + """Reject aliases, expressions, other versions, and missing declarations.""" workflow_paths = _workflow_paths() self.assertTrue(workflow_paths, "Orgmetra must keep repository-owned workflows") @@ -155,7 +251,7 @@ def test_all_repository_workflow_runner_selectors_are_exact(self) -> None: ) def test_runner_parser_rejects_dynamic_and_noncanonical_values(self) -> None: - """Keep the validator sensitive to aliases, expressions, lists, and other images.""" + """Keep the validator sensitive to aliases, expressions, lists, and versions.""" sample = "\n".join( ( "runs-on: ubuntu-latest", @@ -182,7 +278,7 @@ def test_runner_parser_rejects_dynamic_and_noncanonical_values(self) -> None: ) def test_runner_parser_only_strips_yaml_comment_tokens(self) -> None: - """Do not mistake a hash inside a plain or quoted scalar for a YAML comment.""" + """Do not mistake a hash inside a plain or quoted scalar for a comment.""" sample = "\n".join( ( "runs-on: ubuntu-24.04 # supported image", @@ -201,7 +297,7 @@ def test_runner_parser_only_strips_yaml_comment_tokens(self) -> None: class GitHubActionsActionPinningContractTest(unittest.TestCase): - """Keep every remote action reference pinned to an immutable commit.""" + """Keep remote action and image dependencies immutable.""" def test_all_remote_action_references_are_commit_pinned(self) -> None: """Reject mutable tags and branches such as ``@v4`` or ``@main``.""" @@ -220,7 +316,7 @@ def test_all_remote_action_references_are_commit_pinned(self) -> None: ) def test_action_parser_rejects_mutable_and_pins_commit_sha(self) -> None: - """Keep the validator sensitive to tags, branches, and local/Docker refs.""" + """Cover standalone/list uses, repository subpaths, and local exemptions.""" sample = "\n".join( ( "- uses: actions/checkout@v4", @@ -251,11 +347,6 @@ def test_action_parser_rejects_mutable_and_pins_commit_sha(self) -> None: ], [value for _, value in declarations], ) - unpinned = [ - value - for _, value in declarations - if not _PINNED_ACTION_PATTERN.match(value) - ] self.assertEqual( [ "actions/checkout@v4", @@ -263,11 +354,15 @@ def test_action_parser_rejects_mutable_and_pins_commit_sha(self) -> None: "actions/checkout@main", "owner/repo/.github/workflows/ci.yml@v1", ], - unpinned, + [ + value + for _, value in declarations + if not _PINNED_ACTION_PATTERN.match(value) + ], ) def test_all_container_images_are_digest_pinned(self) -> None: - """Reject mutable container/service tags such as ``postgres:16``.""" + """Reject mutable job-container and service-image tags.""" unpinned: list[str] = [] for workflow_path in _workflow_paths(): for line_number, value in _image_declarations( @@ -283,7 +378,7 @@ def test_all_container_images_are_digest_pinned(self) -> None: ) def test_image_parser_rejects_mutable_tags_and_pins_digest(self) -> None: - """Keep the validator sensitive to tags, expressions, and pinned digests.""" + """Cover object images, scalar job containers, expressions, and digests.""" pinned_image = ( "postgres:17.6-alpine@sha256:" "ef257d85f76e48da1c64832459b59fcaba1a4dac97bf5d7450c77753542eee94" @@ -294,6 +389,10 @@ def test_image_parser_rejects_mutable_tags_and_pins_digest(self) -> None: f"image: {pinned_image}", "image: ${{ matrix.image }}", f"image: '{pinned_image}' # latest", + "container: ghcr.io/example/app:latest", + f"container: {pinned_image}", + "container:", + f" image: {pinned_image}", ) ) declarations = _image_declarations(sample) @@ -303,136 +402,125 @@ def test_image_parser_rejects_mutable_tags_and_pins_digest(self) -> None: pinned_image, "${{ matrix.image }}", pinned_image, + "ghcr.io/example/app:latest", + pinned_image, + pinned_image, ], [value for _, value in declarations], ) - unpinned = [ - value for _, value in declarations if not _PINNED_IMAGE_PATTERN.match(value) - ] self.assertEqual( - ["postgres:17.6-alpine", "${{ matrix.image }}"], - unpinned, + ["postgres:17.6-alpine", "${{ matrix.image }}", "ghcr.io/example/app:latest"], + [ + value + for _, value in declarations + if not _PINNED_IMAGE_PATTERN.match(value) + ], ) -def _declared_permission_blocks(workflow: str) -> list[dict[str, str]]: - """Return every ``permissions`` block, workflow-level and job-level. - - GitHub replaces rather than merges the workflow default when a job declares - its own ``permissions:`` block, so a read-only workflow default can be - escalated per job; every block is inspected. - """ - blocks: list[dict[str, str]] = [] - lines = workflow.splitlines() - for index, line in enumerate(lines): - if _PERMISSIONS_BLOCK_PATTERN.match(_strip_yaml_comment(line)) is None: - continue - scopes: dict[str, str] = {} - for child in lines[index + 1 :]: - if child.strip() == "" or child.lstrip().startswith("#"): - continue - match = _PERMISSION_SCOPE_PATTERN.match(_strip_yaml_comment(child)) - if match is None: - break - scopes[match.group(1)] = match.group(2) - blocks.append(scopes) - return blocks - - -def _declared_permissions(workflow: str) -> dict[str, str]: - """Return the workflow-level ``permissions`` block as a scope→level mapping. - - A job-level override cannot loosen the repository-wide default that gates - the whole workflow run, so only the unindented top-level block is returned - here; ``_declared_permission_blocks`` inspects job-level blocks too. - """ - scopes: dict[str, str] = {} - lines = workflow.splitlines() - for index, line in enumerate(lines): - if line != line.lstrip(): - continue - if _PERMISSIONS_BLOCK_PATTERN.match(_strip_yaml_comment(line)) is None: - continue - for child in lines[index + 1 :]: - if child.strip() == "" or child.lstrip().startswith("#"): - continue - match = _PERMISSION_SCOPE_PATTERN.match(_strip_yaml_comment(child)) - if match is None: - break - scopes[match.group(1)] = match.group(2) - break - return scopes - - class GitHubActionsLeastPrivilegeContractTest(unittest.TestCase): """Keep repository-owned workflows read-only and off privileged triggers.""" def test_local_workflows_declare_only_read_scoped_contents_permission(self) -> None: - """Reject missing, write-scoped, or broadened workflow and job permissions.""" + """Require safe workflow defaults and reject unsafe job overrides.""" missing: list[str] = [] violations: list[str] = [] for workflow_path in _workflow_paths(): workflow = workflow_path.read_text(encoding="utf-8") - if not _declared_permissions(workflow): + if _declared_permissions(workflow) != {"contents": "read"}: missing.append(workflow_path.name) - continue - for scopes in _declared_permission_blocks(workflow): - for scope, level in scopes.items(): - if scope != "contents" or level.lower() != "read": - violations.append(f"{workflow_path.name}: {scope}={level}") - self.assertEqual([], missing, f"workflow-level permissions block is missing from: {missing}") + for violation in _permission_violations(workflow): + violations.append(f"{workflow_path.name}: {violation}") + + self.assertEqual( + [], + missing, + f"top-level permissions must declare only contents: read: {missing}", + ) self.assertEqual( [], violations, - f"local workflows must grant only contents: read in every block: {violations}", + f"workflow/job permissions must grant only contents: read: {violations}", ) def test_local_workflows_reject_privileged_triggers_and_write_all(self) -> None: - """Reject pull_request_target, workflow_run, and write-all escalation.""" + """Reject privileged events in mapping/shorthand syntax and write-all.""" violations: list[str] = [] for workflow_path in _workflow_paths(): workflow = workflow_path.read_text(encoding="utf-8") + for line_number, value in _privileged_trigger_declarations(workflow): + violations.append( + f"{workflow_path.name}:{line_number}=privileged trigger {value!r}" + ) for line_number, line in enumerate(workflow.splitlines(), start=1): candidate = _strip_yaml_comment(line) - if _PRIVILEGED_TRIGGER_PATTERN.match(candidate): - violations.append(f"{workflow_path.name}:{line_number}={line.strip()!r}") if _WRITE_ALL_PATTERN.search(candidate): - violations.append(f"{workflow_path.name}:{line_number}={line.strip()!r}") + violations.append( + f"{workflow_path.name}:{line_number}={line.strip()!r}" + ) self.assertEqual( [], violations, f"privileged triggers and write-all permissions are forbidden: {violations}", ) - def test_least_privilege_parser_is_sensitive_to_unsafe_workflows(self) -> None: - """Keep the permission parser and trigger guard fail-closed.""" - self.assertEqual({"contents": "read"}, _declared_permissions("permissions:\n contents: read\n")) + def test_permission_parser_preserves_scope_and_rejects_job_escalation(self) -> None: + """Do not confuse job-level permissions with a workflow-level default.""" + self.assertEqual( + {"contents": "read"}, + _declared_permissions("permissions:\n contents: read\n"), + ) self.assertEqual( {"contents": "write", "id-token": "write"}, - _declared_permissions("permissions:\n contents: write\n id-token: write\n"), + _declared_permissions( + "permissions:\n contents: write\n id-token: write\n" + ), ) self.assertEqual({}, _declared_permissions("name: no permissions here\n")) self.assertEqual( {}, - _declared_permissions("jobs:\n test:\n permissions:\n contents: read\n"), + _declared_permissions( + "jobs:\n test:\n permissions:\n contents: read\n" + ), ) + + safe = ( + "permissions:\n" + " contents: read\n" + "jobs:\n" + " test:\n" + " permissions:\n" + " contents: read\n" + ) + unsafe = safe.replace( + " contents: read\n", + " contents: write\n id-token: write\n", + ) + self.assertEqual([], _permission_violations(safe)) + self.assertNotEqual([], _permission_violations(unsafe)) + self.assertNotEqual( + [], + _permission_violations("permissions: write-all\n"), + ) + + def test_trigger_parser_covers_mapping_scalar_and_flow_forms(self) -> None: + """Detect privileged events in every GitHub-supported ``on`` shorthand.""" self.assertEqual( - [{"contents": "read"}], - _declared_permission_blocks("permissions:\n contents: read\n"), + [(2, "pull_request_target")], + _privileged_trigger_declarations("on:\n pull_request_target:\n"), ) self.assertEqual( - [{"contents": "read"}, {"contents": "write"}], - _declared_permission_blocks( - "permissions:\n contents: read\njobs:\n build:\n permissions:\n contents: write\n" - ), + [(1, "pull_request_target")], + _privileged_trigger_declarations("on: pull_request_target\n"), + ) + self.assertEqual( + [(1, "[push, workflow_run]")], + _privileged_trigger_declarations("on: [push, workflow_run]\n"), + ) + self.assertEqual( + [], + _privileged_trigger_declarations("on: [push, pull_request]\n"), ) - self.assertEqual([], _declared_permission_blocks("name: no permissions here\n")) - self.assertIsNotNone(_PRIVILEGED_TRIGGER_PATTERN.match("pull_request_target:")) - self.assertIsNotNone(_PRIVILEGED_TRIGGER_PATTERN.match(" pull_request_target:")) - self.assertIsNotNone(_PRIVILEGED_TRIGGER_PATTERN.match("workflow_run:")) - self.assertIsNone(_PRIVILEGED_TRIGGER_PATTERN.match("pull_request:")) - self.assertIsNotNone(_WRITE_ALL_PATTERN.search("permissions: write-all")) - self.assertIsNone(_WRITE_ALL_PATTERN.search("permissions:\n contents: read")) class GitHubActionsQueueContractTest(unittest.TestCase): @@ -468,7 +556,7 @@ def test_foundation_workflow_expands_to_one_job(self) -> None: self.assertNotIn("matrix:", jobs) def test_postgres_contracts_wait_on_the_dynamic_host_port(self) -> None: - """Prove Docker port forwarding is usable before running each database contract.""" + """Prove Docker port forwarding is usable before each database contract.""" workflow = (WORKFLOWS / "foundation-ci.yml").read_text(encoding="utf-8") dynamic_publish = "--publish 127.0.0.1::5432" port_lookup = 'postgres_binding="$(docker port "$container_name" 5432/tcp)"' @@ -485,5 +573,5 @@ def test_postgres_contracts_wait_on_the_dynamic_host_port(self) -> None: self.assertLess(workflow.index(host_probe), workflow.index(contract_run)) -if __name__ == "__main__": # pragma: no cover - normal execution is via unittest discovery. - unittest.main() \ No newline at end of file +if __name__ == "__main__": # pragma: no cover + unittest.main() From c5fe34cd1ae4dd3a8a6b66cafc27ef82af6a7b38 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 11 Sep 2026 13:40:29 +0900 Subject: [PATCH 11/12] fix(ci): normalize quoted workflow mapping keys --- tests/test_github_actions_runner_image.py | 182 ++++++++++++++++------ 1 file changed, 132 insertions(+), 50 deletions(-) diff --git a/tests/test_github_actions_runner_image.py b/tests/test_github_actions_runner_image.py index 3717d9e67..e0eb0c738 100644 --- a/tests/test_github_actions_runner_image.py +++ b/tests/test_github_actions_runner_image.py @@ -15,18 +15,14 @@ ROOT = Path(__file__).resolve().parents[1] WORKFLOWS = ROOT / ".github" / "workflows" -_RUNS_ON_PATTERN = re.compile(r"^\s*runs-on\s*:\s*(.*?)\s*$") -_USES_PATTERN = re.compile(r"^\s*(?:-\s+)?uses\s*:\s*(.*?)\s*$") -_IMAGE_PATTERN = re.compile(r"^\s*image\s*:\s*(.*?)\s*$") -_CONTAINER_PATTERN = re.compile(r"^\s*container\s*:\s*(.*?)\s*$") -_PERMISSIONS_BLOCK_PATTERN = re.compile(r"^permissions\s*:\s*(.*?)\s*$") -_PERMISSION_SCOPE_PATTERN = re.compile(r"^([A-Za-z0-9-]+)\s*:\s*(read|write|none)\s*$") -_ON_PATTERN = re.compile(r"^on\s*:\s*(.*?)\s*$") -_PRIVILEGED_TRIGGER_KEY_PATTERN = re.compile(r"^(pull_request_target|workflow_run)\s*:") +_MAPPING_ENTRY_PATTERN = re.compile( + r"^(?P-\s+)?" + r"(?P[A-Za-z0-9_-]+|'(?:[^']|'')*'|\"(?:[^\"\\]|\\.)*\")" + r"\s*:\s*(?P.*?)\s*$" +) _PRIVILEGED_EVENT_PATTERN = re.compile( r"(? str: return value +def _mapping_entry(line: str) -> tuple[int, bool, str, str] | None: + """Return indentation, sequence form, key, and scalar for one YAML mapping entry.""" + indent = len(line) - len(line.lstrip()) + candidate = _strip_yaml_comment(line.lstrip()) + match = _MAPPING_ENTRY_PATTERN.match(candidate) + if match is None: + return None + return ( + indent, + match.group("sequence") is not None, + _unquote_scalar(match.group("key")), + _unquote_scalar(match.group("value")), + ) + + def _runner_declarations(workflow: str) -> list[tuple[int, str]]: """Return line-numbered scalar ``runs-on`` declarations.""" declarations: list[tuple[int, str]] = [] for line_number, line in enumerate(workflow.splitlines(), start=1): - match = _RUNS_ON_PATTERN.match(line) - if match is not None: - declarations.append( - (line_number, _unquote_scalar(_strip_yaml_comment(match.group(1)))) - ) + entry = _mapping_entry(line) + if entry is None: + continue + _indent, sequence, key, value = entry + if not sequence and key == "runs-on": + declarations.append((line_number, value)) return declarations @@ -108,10 +120,12 @@ def _action_declarations(workflow: str) -> list[tuple[int, str]]: """Return remote action/reusable-workflow ``uses`` declarations.""" declarations: list[tuple[int, str]] = [] for line_number, line in enumerate(workflow.splitlines(), start=1): - match = _USES_PATTERN.match(line) - if match is None: + entry = _mapping_entry(line) + if entry is None: + continue + _indent, _sequence, key, value = entry + if key != "uses": continue - value = _unquote_scalar(_strip_yaml_comment(match.group(1))) if value.startswith("./") or value.startswith("docker://"): continue declarations.append((line_number, value)) @@ -122,13 +136,13 @@ def _image_declarations(workflow: str) -> list[tuple[int, str]]: """Return image references from ``image:`` and scalar ``container:`` forms.""" declarations: list[tuple[int, str]] = [] for line_number, line in enumerate(workflow.splitlines(), start=1): - match = _IMAGE_PATTERN.match(line) - if match is None: - match = _CONTAINER_PATTERN.match(line) - if match is None: + entry = _mapping_entry(line) + if entry is None: + continue + _indent, _sequence, key, value = entry + if key not in {"image", "container"}: continue - value = _unquote_scalar(_strip_yaml_comment(match.group(1))) # ``container:`` with no scalar value starts the object form; its nested # ``image:`` member is collected on its own line. if not value: @@ -145,27 +159,30 @@ def _permission_blocks( blocks: list[tuple[int, int, str, dict[str, str]]] = [] for index, line in enumerate(lines): - candidate = _strip_yaml_comment(line) - match = _PERMISSIONS_BLOCK_PATTERN.match(candidate) - if match is None: + entry = _mapping_entry(line) + if entry is None: + continue + indent, sequence, key, scalar = entry + if sequence or key != "permissions": continue - indent = len(line) - len(line.lstrip()) - scalar = _unquote_scalar(match.group(1)) scopes: dict[str, str] = {} if not scalar: for child in lines[index + 1 :]: if child.strip() == "" or child.lstrip().startswith("#"): continue + child_entry = _mapping_entry(child) child_indent = len(child) - len(child.lstrip()) if child_indent <= indent: break - child_candidate = _strip_yaml_comment(child) - scope_match = _PERMISSION_SCOPE_PATTERN.match(child_candidate) - if scope_match is None: - scopes[""] = child_candidate + if child_entry is None: + scopes[""] = _strip_yaml_comment(child) + break + _child_indent, child_sequence, child_key, child_value = child_entry + if child_sequence or child_value not in {"read", "write", "none"}: + scopes[""] = _strip_yaml_comment(child) break - scopes[scope_match.group(1)] = scope_match.group(2) + scopes[child_key] = child_value blocks.append((index + 1, indent, scalar, scopes)) return blocks @@ -201,25 +218,22 @@ def _privileged_trigger_declarations(workflow: str) -> list[tuple[int, str]]: if line.strip() == "" or line.lstrip().startswith("#"): continue - indent = len(line) - len(line.lstrip()) - candidate = _strip_yaml_comment(line) + entry = _mapping_entry(line) + if entry is None: + continue + indent, sequence, key, value = entry - if indent == 0: - on_match = _ON_PATTERN.match(candidate) - if on_match is not None: - value = _unquote_scalar(on_match.group(1)) - on_block_indent = 0 if not value else None - if value and _PRIVILEGED_EVENT_PATTERN.search(value): - declarations.append((line_number, value)) - continue + if indent == 0 and not sequence and key == "on": + on_block_indent = 0 if not value else None + if value and _PRIVILEGED_EVENT_PATTERN.search(value): + declarations.append((line_number, value)) + continue if on_block_indent is not None: if indent <= on_block_indent: on_block_indent = None - else: - trigger_match = _PRIVILEGED_TRIGGER_KEY_PATTERN.match(candidate) - if trigger_match is not None: - declarations.append((line_number, trigger_match.group(1))) + elif not sequence and key in {"pull_request_target", "workflow_run"}: + declarations.append((line_number, key)) return declarations @@ -361,6 +375,30 @@ def test_action_parser_rejects_mutable_and_pins_commit_sha(self) -> None: ], ) + def test_action_parser_normalizes_quoted_mapping_keys(self) -> None: + """Treat bare and quoted ``uses`` keys as the same security declaration.""" + pinned = "actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1" + sample = "\n".join( + ( + '"uses": actions/checkout@v4', + "- 'uses': actions/checkout@main", + f"'uses': {pinned}", + ) + ) + declarations = _action_declarations(sample) + self.assertEqual( + ["actions/checkout@v4", "actions/checkout@main", pinned], + [value for _, value in declarations], + ) + self.assertEqual( + ["actions/checkout@v4", "actions/checkout@main"], + [ + value + for _, value in declarations + if not _PINNED_ACTION_PATTERN.match(value) + ], + ) + def test_all_container_images_are_digest_pinned(self) -> None: """Reject mutable job-container and service-image tags.""" unpinned: list[str] = [] @@ -417,6 +455,24 @@ def test_image_parser_rejects_mutable_tags_and_pins_digest(self) -> None: ], ) + def test_image_parser_normalizes_quoted_mapping_keys(self) -> None: + """Treat quoted image/container keys as equivalent immutable inputs.""" + pinned_image = ( + "postgres:17.6-alpine@sha256:" + "ef257d85f76e48da1c64832459b59fcaba1a4dac97bf5d7450c77753542eee94" + ) + sample = "\n".join( + ( + '"image": postgres:latest', + "'container': ghcr.io/example/app:latest", + f" 'image': '{pinned_image}'", + ) + ) + self.assertEqual( + ["postgres:latest", "ghcr.io/example/app:latest", pinned_image], + [value for _, value in _image_declarations(sample)], + ) + class GitHubActionsLeastPrivilegeContractTest(unittest.TestCase): """Keep repository-owned workflows read-only and off privileged triggers.""" @@ -452,11 +508,10 @@ def test_local_workflows_reject_privileged_triggers_and_write_all(self) -> None: violations.append( f"{workflow_path.name}:{line_number}=privileged trigger {value!r}" ) - for line_number, line in enumerate(workflow.splitlines(), start=1): - candidate = _strip_yaml_comment(line) - if _WRITE_ALL_PATTERN.search(candidate): + for line_number, _indent, scalar, _scopes in _permission_blocks(workflow): + if scalar == "write-all": violations.append( - f"{workflow_path.name}:{line_number}={line.strip()!r}" + f"{workflow_path.name}:{line_number}=permissions: write-all" ) self.assertEqual( [], @@ -503,6 +558,22 @@ def test_permission_parser_preserves_scope_and_rejects_job_escalation(self) -> N _permission_violations("permissions: write-all\n"), ) + def test_permission_parser_normalizes_quoted_mapping_keys(self) -> None: + """Reject quoted job escalation while accepting quoted read-only defaults.""" + workflow = ( + '"permissions":\n' + ' "contents": read\n' + "jobs:\n" + " test:\n" + " 'permissions':\n" + " 'contents': write\n" + ) + self.assertEqual({"contents": "read"}, _declared_permissions(workflow)) + self.assertEqual( + ["line 5: scopes={'contents': 'write'}"], + _permission_violations(workflow), + ) + def test_trigger_parser_covers_mapping_scalar_and_flow_forms(self) -> None: """Detect privileged events in every GitHub-supported ``on`` shorthand.""" self.assertEqual( @@ -522,6 +593,17 @@ def test_trigger_parser_covers_mapping_scalar_and_flow_forms(self) -> None: _privileged_trigger_declarations("on: [push, pull_request]\n"), ) + def test_trigger_parser_normalizes_quoted_mapping_keys(self) -> None: + """Detect privileged scalar and mapping events when YAML keys are quoted.""" + self.assertEqual( + [(1, "pull_request_target")], + _privileged_trigger_declarations('"on": pull_request_target\n'), + ) + self.assertEqual( + [(2, "workflow_run")], + _privileged_trigger_declarations("'on':\n 'workflow_run':\n"), + ) + class GitHubActionsQueueContractTest(unittest.TestCase): """Keep local workflows bounded and same-PR cancellation isolated.""" From a50e591d6b3fd690e830399b7920c383aaa6975e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 11 Sep 2026 15:06:53 +0900 Subject: [PATCH 12/12] fix(ci): require digest-pinned Docker actions --- tests/test_github_actions_runner_image.py | 48 +++++++++++++++++++---- 1 file changed, 40 insertions(+), 8 deletions(-) diff --git a/tests/test_github_actions_runner_image.py b/tests/test_github_actions_runner_image.py index e0eb0c738..a312eee62 100644 --- a/tests/test_github_actions_runner_image.py +++ b/tests/test_github_actions_runner_image.py @@ -117,7 +117,7 @@ def _runner_declarations(workflow: str) -> list[tuple[int, str]]: def _action_declarations(workflow: str) -> list[tuple[int, str]]: - """Return remote action/reusable-workflow ``uses`` declarations.""" + """Return non-local action/reusable-workflow ``uses`` declarations.""" declarations: list[tuple[int, str]] = [] for line_number, line in enumerate(workflow.splitlines(), start=1): entry = _mapping_entry(line) @@ -126,12 +126,19 @@ def _action_declarations(workflow: str) -> list[tuple[int, str]]: _indent, _sequence, key, value = entry if key != "uses": continue - if value.startswith("./") or value.startswith("docker://"): + if value.startswith("./"): continue declarations.append((line_number, value)) return declarations +def _action_reference_is_immutable(value: str) -> bool: + """Require repository actions to pin commits and Docker actions to pin digests.""" + if value.startswith("docker://"): + return _PINNED_IMAGE_PATTERN.fullmatch(value.removeprefix("docker://")) is not None + return _PINNED_ACTION_PATTERN.fullmatch(value) is not None + + def _image_declarations(workflow: str) -> list[tuple[int, str]]: """Return image references from ``image:`` and scalar ``container:`` forms.""" declarations: list[tuple[int, str]] = [] @@ -313,19 +320,19 @@ def test_runner_parser_only_strips_yaml_comment_tokens(self) -> None: class GitHubActionsActionPinningContractTest(unittest.TestCase): """Keep remote action and image dependencies immutable.""" - def test_all_remote_action_references_are_commit_pinned(self) -> None: - """Reject mutable tags and branches such as ``@v4`` or ``@main``.""" + def test_all_remote_action_references_are_immutable(self) -> None: + """Reject mutable repository refs and mutable Docker action tags.""" unpinned: list[str] = [] for workflow_path in _workflow_paths(): for line_number, value in _action_declarations( workflow_path.read_text(encoding="utf-8") ): - if not _PINNED_ACTION_PATTERN.match(value): + if not _action_reference_is_immutable(value): unpinned.append(f"{workflow_path.name}:{line_number}={value!r}") self.assertEqual( [], unpinned, - "remote action references must pin a full 40-character commit SHA: " + "remote actions must pin a full commit SHA or Docker image sha256 digest: " f"{unpinned}", ) @@ -354,6 +361,7 @@ def test_action_parser_rejects_mutable_and_pins_commit_sha(self) -> None: "actions/checkout@main", "actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1", "actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97", + "docker://alpine:3.20", "actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97", "ContextualWisdomLab/Orgmetra/.github/workflows/ci.yml@3d3c42e5aac5ba805825da76410c181273ba90b1", "owner/repo/sub/dir@5fda3b95a4ea91299a34e894583c3862153e4b97", @@ -366,12 +374,13 @@ def test_action_parser_rejects_mutable_and_pins_commit_sha(self) -> None: "actions/checkout@v4", "actions/checkout@v4", "actions/checkout@main", + "docker://alpine:3.20", "owner/repo/.github/workflows/ci.yml@v1", ], [ value for _, value in declarations - if not _PINNED_ACTION_PATTERN.match(value) + if not _action_reference_is_immutable(value) ], ) @@ -395,10 +404,33 @@ def test_action_parser_normalizes_quoted_mapping_keys(self) -> None: [ value for _, value in declarations - if not _PINNED_ACTION_PATTERN.match(value) + if not _action_reference_is_immutable(value) ], ) + def test_docker_action_references_require_digest(self) -> None: + """Treat Docker ``uses`` as remote executable dependencies, not local actions.""" + digest = "ef257d85f76e48da1c64832459b59fcaba1a4dac97bf5d7450c77753542eee94" + untagged = f"docker://alpine@sha256:{digest}" + tagged = f"docker://alpine:3.20@sha256:{digest}" + sample = "\n".join( + ( + "uses: docker://alpine:3.20", + f'"uses": {untagged}', + f"- 'uses': {tagged}", + "uses: ./local-action", + ) + ) + declarations = _action_declarations(sample) + self.assertEqual( + ["docker://alpine:3.20", untagged, tagged], + [value for _, value in declarations], + ) + self.assertEqual( + [False, True, True], + [_action_reference_is_immutable(value) for _, value in declarations], + ) + def test_all_container_images_are_digest_pinned(self) -> None: """Reject mutable job-container and service-image tags.""" unpinned: list[str] = []