Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GitHub OIDC: validate job_workflow_ref #11263

Merged
merged 9 commits into from
Apr 28, 2022
33 changes: 33 additions & 0 deletions tests/unit/oidc/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
# limitations under the License.

import pretend
import pytest

from warehouse.oidc import models

Expand Down Expand Up @@ -132,3 +133,35 @@ def test_github_provider_verifies(self, monkeypatch):
}
assert provider.verify_claims(signed_claims=signed_claims)
assert len(noop_check.calls) == len(verifiable_claims)

@pytest.mark.parametrize(
("claim", "valid"),
[
# okay: workflow name, followed by a nonempty tail
("foo/bar/.github/workflows/baz.yml@refs/tags/v0.0.1", True),
("foo/bar/.github/workflows/baz.yml@refs/pulls/6", True),
("foo/bar/.github/workflows/baz.yml@refs/heads/main", True),
("foo/bar/.github/workflows/baz.yml@notrailingslash", True),
# bad: workflow name, followed by an attempted impersonator
("foo/bar/.github/workflows/baz.yml@fake.yml@notrailingslash", False),
("foo/bar/.github/workflows/baz.yml@fake.yml@refs/pulls/6", False),
# bad: missing tail or workflow name or otherwise partial
("foo/bar/.github/workflows/baz.yml@", False),
("foo/bar/.github/workflows/@", False),
("foo/bar/.github/workflows/", False),
("baz.yml", False),
("foo/bar/.github/workflows/baz.yml@malicious.yml@", False),
("foo/bar/.github/workflows/baz.yml@@", False),
("", False),
],
)
def test_github_provider_job_workflow_ref(self, claim, valid):
provider = models.GitHubProvider(
repository_name="bar",
repository_owner="foo",
repository_owner_id=pretend.stub(),
workflow_filename="baz.yml",
)

check = models.GitHubProvider.__verifiable_claims__["job_workflow_ref"]
assert check(provider.job_workflow_ref, claim) is valid
25 changes: 20 additions & 5 deletions warehouse/oidc/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,22 @@
from warehouse.packaging.models import Project


def _check_job_workflow_ref(ground_truth, signed_claim):
# We expect a string formatted as follows:
# OWNER/REPO/.github/workflows/WORKFLOW.yml@TAIL
# where TAIL might be a ref (`refs/...`) or a commit hash,
# but is always nonempty.
try:
repo_workflow_path, tail = signed_claim.rsplit("@", 1)
except ValueError:
return False

if not tail:
return False

return repo_workflow_path == ground_truth


class OIDCProviderProjectAssociation(db.Model):
__tablename__ = "oidc_provider_project_association"

Expand Down Expand Up @@ -146,9 +162,9 @@ class GitHubProvider(OIDCProvider):

__verifiable_claims__ = {
"repository": str.__eq__,
"workflow": str.__eq__,
"repository_owner": str.__eq__,
"repository_owner_id": str.__eq__,
"job_workflow_ref": _check_job_workflow_ref,
}

__unchecked_claims__ = {
Expand All @@ -166,8 +182,7 @@ class GitHubProvider(OIDCProvider):
"event_name",
"ref_type",
"repository_id",
# TODO(#11096): Support reusable workflows.
"job_workflow_ref",
"workflow",
}

@property
Expand All @@ -179,8 +194,8 @@ def repository(self):
return f"{self.repository_owner}/{self.repository_name}"

@property
def workflow(self):
return self.workflow_filename
def job_workflow_ref(self):
return f"{self.repository}/.github/workflows/{self.workflow_filename}"

def __str__(self):
return f"{self.workflow_filename} @ {self.repository}"