From 8626da35b2464e098914b8145c8afad7cd8ed346 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 25 Jul 2026 18:05:56 +0000 Subject: [PATCH] Allow explicit GitHub organization in destination config Support organization-only, organization+repo-name, and owner/repo shorthand. Reject combining organization with owner/repo (redundant or mismatched). Repo/env operations still require a concrete repository. Co-authored-by: Abhishek Chadha --- README.md | 4 + src/secretsync/destinations/github_actions.py | 184 ++++++++++++++++-- src/secretsync/init_templates.py | 4 + tests/integration/test_github_actions.py | 118 ++++++++++- 4 files changed, 296 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 497c87a..bb68662 100644 --- a/README.md +++ b/README.md @@ -54,7 +54,11 @@ sets: destinations: github: connector: github-actions + # Either repository: owner/name … repository: owner/repo + # … or organization: + repository: name (not both with owner/name) + # organization: owner + # repository: repo auth: tokenEnv: GITHUB_TOKEN # reads os.environ["GITHUB_TOKEN"] for API auth diff --git a/src/secretsync/destinations/github_actions.py b/src/secretsync/destinations/github_actions.py index 6c5e92b..35d2302 100644 --- a/src/secretsync/destinations/github_actions.py +++ b/src/secretsync/destinations/github_actions.py @@ -33,8 +33,25 @@ GITHUB_API = "https://api.github.com" SECRET_NAME_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") -REPO_RE = re.compile(r"^[^/\s]+/[^/\s]+$") +OWNER_REPO_RE = re.compile(r"^[^/\s]+/[^/\s]+$") +SEGMENT_RE = re.compile(r"^[^/\s]+$") VARIABLE_NAME_RE = SECRET_NAME_RE +# Backward-compatible alias used by older tests / callers. +REPO_RE = OWNER_REPO_RE + + +@dataclass(frozen=True, slots=True) +class GitHubRepoRef: + """Resolved GitHub owner + optional repository name.""" + + owner: str + repository: str | None = None + + +@dataclass(frozen=True, slots=True) +class _ConfigParseError: + message: str + hint: str | None = None def _capabilities() -> DestinationCapabilities: @@ -56,12 +73,139 @@ def encrypt_github_secret(public_key_b64: str, secret_value: bytes) -> str: return base64.b64encode(sealed).decode("utf-8") +def _parse_github_target( + config: Mapping[str, JsonValue], +) -> GitHubRepoRef | _ConfigParseError: + """Resolve destination organization/repository into an owner (+ optional repo). + + Allowed: + - organization: org + - organization: org + repository: name (name must not contain '/') + - repository: owner/name (no organization field) + + Rejected: + - organization + repository: owner/name (redundant or mismatched) + - repository: name without organization + - neither field set + """ + raw_org = config.get("organization") + raw_repo = config.get("repository") + + org: str | None = None + if raw_org is not None: + if not isinstance(raw_org, str) or not raw_org.strip(): + return _ConfigParseError( + message="github-actions organization must be a non-empty string", + hint="Example: organization: my-org", + ) + org = raw_org.strip() + if not SEGMENT_RE.match(org): + return _ConfigParseError( + message=( + f"github-actions organization {org!r} must be a single path segment (no '/')" + ), + hint="Use organization: my-org — not an owner/repo string.", + ) + + repo: str | None = None + if raw_repo is not None: + if not isinstance(raw_repo, str) or not raw_repo.strip(): + return _ConfigParseError( + message="github-actions repository must be a non-empty string", + hint="Use repository: owner/name, or organization: + repository: name.", + ) + repo = raw_repo.strip() + + if org is None and repo is None: + return _ConfigParseError( + message="github-actions requires organization and/or repository", + hint=( + "Set organization: my-org, or repository: owner/name, " + "or both as organization: my-org and repository: my-repo." + ), + ) + + if org is not None and repo is None: + return GitHubRepoRef(owner=org, repository=None) + + assert repo is not None + + if org is not None: + if "/" in repo: + if OWNER_REPO_RE.match(repo): + owner, _name = repo.split("/", 1) + if owner == org: + return _ConfigParseError( + message=( + "github-actions must not set organization together with " + "repository as 'owner/name'" + ), + hint=( + f"Use repository: {_name!r} with organization: {org!r}, " + f"or drop organization and keep repository: {repo!r}." + ), + ) + return _ConfigParseError( + message=( + f"github-actions organization {org!r} does not match " + f"repository owner {owner!r} in {repo!r}" + ), + hint=( + f"Use organization: {owner!r} with repository: " + f"{repo.split('/', 1)[1]!r}, or repository: {repo!r} alone." + ), + ) + return _ConfigParseError( + message=f"github-actions repository {repo!r} is invalid", + hint="With organization set, repository must be a bare repo name (no '/').", + ) + if not SEGMENT_RE.match(repo): + return _ConfigParseError( + message=f"github-actions repository {repo!r} is invalid", + hint="With organization set, repository must be a bare repo name (no '/').", + ) + return GitHubRepoRef(owner=org, repository=repo) + + # repository alone — must be owner/name + if not OWNER_REPO_RE.match(repo): + return _ConfigParseError( + message=( + f"github-actions repository {repo!r} must be 'owner/name' " + "when organization is omitted" + ), + hint=("Use repository: owner/name, or set organization: owner and repository: name."), + ) + owner, name = repo.split("/", 1) + return GitHubRepoRef(owner=owner, repository=name) + + def _parse_repository(config: Mapping[str, JsonValue]) -> tuple[str, str] | None: - raw = config.get("repository") - if not isinstance(raw, str) or not REPO_RE.match(raw): + """Return (owner, repo) when the destination resolves to a concrete repository.""" + parsed = _parse_github_target(config) + if isinstance(parsed, _ConfigParseError) or parsed.repository is None: return None - owner, repo = raw.split("/", 1) - return owner, repo + return parsed.owner, parsed.repository + + +def _require_repo_ref( + config: Mapping[str, JsonValue], +) -> tuple[str, str] | _ConfigParseError: + """Like _parse_github_target, but require a concrete repository for repo/env ops.""" + parsed = _parse_github_target(config) + if isinstance(parsed, _ConfigParseError): + return parsed + if parsed.repository is None: + return _ConfigParseError( + message=( + f"github-actions destination has organization {parsed.owner!r} " + "but no repository; repository/environment scopes require a repository" + ), + hint=( + f"Add repository: under the destination, or use " + f"repository: {parsed.owner}/." + ), + ) + return parsed.owner, parsed.repository def _token_env(config: Mapping[str, JsonValue]) -> str | None: @@ -90,11 +234,13 @@ class GitHubActionsDestination: async def validate(self, config: Mapping[str, JsonValue]) -> list[Issue]: issues: list[Issue] = [] - if _parse_repository(config) is None: + parsed = _parse_github_target(config) + if isinstance(parsed, _ConfigParseError): issues.append( Issue( code="DESTINATION_INVALID", - message="github-actions requires repository as 'owner/name'", + message=parsed.message, + hint=parsed.hint, ) ) if _token_env(config) is None: @@ -118,13 +264,20 @@ async def list_names( *, kind: ValueKind = ValueKind.SECRET, ) -> frozenset[str]: - parsed = _parse_repository(config) + parsed = _require_repo_ref(config) token_env = _token_env(config) - if parsed is None or token_env is None: + if isinstance(parsed, _ConfigParseError) or token_env is None: + message = ( + parsed.message + if isinstance(parsed, _ConfigParseError) + else "Invalid github-actions destination configuration" + ) + hint = parsed.hint if isinstance(parsed, _ConfigParseError) else None raise ListNamesError( SafeConnectorError( code="DESTINATION_INVALID", - message="Invalid github-actions destination configuration", + message=message, + hint=hint, correlation_id=context.correlation_id, ) ) @@ -215,15 +368,20 @@ async def apply( context: OperationContext, ) -> ApplyDestinationResult: config = request.destination_config - parsed = _parse_repository(config) + parsed = _require_repo_ref(config) token_env = _token_env(config) all_ids = [m.mutation_id for m in request.mutations] + [ d.mutation_id for d in request.deletes ] - if parsed is None or token_env is None: + if isinstance(parsed, _ConfigParseError) or token_env is None: error = SafeConnectorError( code="DESTINATION_INVALID", - message="Invalid github-actions destination configuration", + message=( + parsed.message + if isinstance(parsed, _ConfigParseError) + else "Invalid github-actions destination configuration" + ), + hint=parsed.hint if isinstance(parsed, _ConfigParseError) else None, correlation_id=context.correlation_id, ) return ApplyDestinationResult( diff --git a/src/secretsync/init_templates.py b/src/secretsync/init_templates.py index 48e7582..416fd4a 100644 --- a/src/secretsync/init_templates.py +++ b/src/secretsync/init_templates.py @@ -27,6 +27,10 @@ destinations: github: connector: github-actions + # repository: owner/repo + # or: + # organization: owner + # repository: repo repository: owner/repo auth: tokenEnv: GITHUB_TOKEN diff --git a/tests/integration/test_github_actions.py b/tests/integration/test_github_actions.py index 769e70f..83955c9 100644 --- a/tests/integration/test_github_actions.py +++ b/tests/integration/test_github_actions.py @@ -34,7 +34,7 @@ def _mutation( @pytest.mark.asyncio -async def test_validate_requires_repository_and_auth() -> None: +async def test_validate_requires_target_and_auth() -> None: dest = GitHubActionsFactory().create(_services()) issues = await dest.validate({"connector": "github-actions"}) assert any(i.code == "DESTINATION_INVALID" for i in issues) @@ -370,3 +370,119 @@ async def test_delete_variable() -> None: assert route.called assert result.results[0].status == "applied" assert result.results[0].effect == "deleted" + + +def test_parse_github_target_allowed_forms() -> None: + from secretsync.destinations.github_actions import GitHubRepoRef, _parse_github_target + + org_only = _parse_github_target({"organization": "acme"}) + assert org_only == GitHubRepoRef(owner="acme", repository=None) + + org_and_name = _parse_github_target({"organization": "acme", "repository": "web"}) + assert org_and_name == GitHubRepoRef(owner="acme", repository="web") + + shorthand = _parse_github_target({"repository": "acme/web"}) + assert shorthand == GitHubRepoRef(owner="acme", repository="web") + + +def test_parse_github_target_rejects_org_with_owner_repo() -> None: + from secretsync.destinations.github_actions import _ConfigParseError, _parse_github_target + + redundant = _parse_github_target({"organization": "acme", "repository": "acme/web"}) + assert isinstance(redundant, _ConfigParseError) + assert "must not set organization together" in redundant.message + assert redundant.hint is not None + assert "repository: 'web'" in redundant.hint + + mismatched = _parse_github_target({"organization": "acme", "repository": "other/web"}) + assert isinstance(mismatched, _ConfigParseError) + assert "does not match" in mismatched.message + assert "other" in mismatched.message + + +def test_parse_github_target_rejects_bare_repo_without_org() -> None: + from secretsync.destinations.github_actions import _ConfigParseError, _parse_github_target + + bare = _parse_github_target({"repository": "web"}) + assert isinstance(bare, _ConfigParseError) + assert "owner/name" in bare.message + + +@pytest.mark.asyncio +async def test_validate_accepts_organization_and_repo_name() -> None: + dest = GitHubActionsFactory().create(_services()) + issues = await dest.validate( + { + "connector": "github-actions", + "organization": "acme", + "repository": "web", + "auth": {"tokenEnv": "GITHUB_TOKEN"}, + } + ) + assert issues == [] + + +@pytest.mark.asyncio +async def test_validate_rejects_organization_with_owner_repo() -> None: + dest = GitHubActionsFactory().create(_services()) + issues = await dest.validate( + { + "connector": "github-actions", + "organization": "acme", + "repository": "acme/web", + "auth": {"tokenEnv": "GITHUB_TOKEN"}, + } + ) + assert any(i.code == "DESTINATION_INVALID" for i in issues) + assert any("must not set organization together" in i.message for i in issues) + + +@pytest.mark.asyncio +@respx.mock +async def test_apply_with_organization_and_repo_name() -> None: + from nacl import encoding + + key = public.PrivateKey.generate().public_key + key_b64 = key.encode(encoder=encoding.Base64Encoder).decode() + respx.get("https://api.github.com/repos/acme/web/actions/secrets/public-key").mock( + return_value=httpx.Response(200, json={"key_id": "key1", "key": key_b64}) + ) + respx.put("https://api.github.com/repos/acme/web/actions/secrets/DATABASE_URL").mock( + return_value=httpx.Response(201) + ) + dest = GitHubActionsFactory().create(_services()) + result = await dest.apply( + ApplyDestinationRequest( + deployment_id="dep", + destination_config={ + "connector": "github-actions", + "organization": "acme", + "repository": "web", + "auth": {"tokenEnv": "GITHUB_TOKEN"}, + }, + mutations=[_mutation("DATABASE_URL")], + ), + OperationContext(correlation_id="c1"), + ) + assert result.results[0].status == "applied" + assert result.results[0].effect == "created" + + +@pytest.mark.asyncio +async def test_org_only_destination_rejects_repo_scope_apply() -> None: + dest = GitHubActionsFactory().create(_services()) + result = await dest.apply( + ApplyDestinationRequest( + deployment_id="dep", + destination_config={ + "connector": "github-actions", + "organization": "acme", + "auth": {"tokenEnv": "GITHUB_TOKEN"}, + }, + mutations=[_mutation("A")], + ), + OperationContext(correlation_id="c1"), + ) + assert result.results[0].status == "failed" + assert result.results[0].error is not None + assert "no repository" in result.results[0].error.message