diff --git a/internal/cli/admin.go b/internal/cli/admin.go index 8047e867fe..8288f57a90 100644 --- a/internal/cli/admin.go +++ b/internal/cli/admin.go @@ -116,9 +116,6 @@ func validateOrgName(org string) error { if strings.HasPrefix(org, "-") || strings.HasSuffix(org, "-") { return fmt.Errorf("organization name cannot start or end with a hyphen") } - if strings.Contains(org, "--") { - return fmt.Errorf("organization name cannot contain consecutive hyphens") - } for _, c := range org { if !((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '-') { return fmt.Errorf("organization name contains invalid character: %c", c) @@ -128,8 +125,8 @@ func validateOrgName(org string) error { } // githubOwnerPattern matches valid GitHub usernames and org names -// (alphanumeric and single hyphens only, no dots or underscores). -var githubOwnerPattern = regexp.MustCompile(`^[a-zA-Z0-9](-?[a-zA-Z0-9])*$`) +// (alphanumeric and hyphens only, no dots or underscores). +var githubOwnerPattern = mintcore.GitHubOrgPattern // githubRepoPattern matches valid GitHub repository names // (alphanumeric, hyphens, dots, and underscores). Dot-prefixed repos such as diff --git a/internal/cli/admin_test.go b/internal/cli/admin_test.go index 6af1a00d59..915b5edb96 100644 --- a/internal/cli/admin_test.go +++ b/internal/cli/admin_test.go @@ -334,7 +334,7 @@ func TestAnalyzeCmd_RequiresOrg(t *testing.T) { } func TestValidateOrgName_Valid(t *testing.T) { - valid := []string{"my-org", "org123", "A", "abc-def-ghi", "ORG"} + valid := []string{"my-org", "my--org", "org123", "A", "abc-def-ghi", "ORG"} for _, name := range valid { t.Run(name, func(t *testing.T) { assert.NoError(t, validateOrgName(name)) diff --git a/internal/cli/inference_test.go b/internal/cli/inference_test.go index eca5c99738..a1f9833f50 100644 --- a/internal/cli/inference_test.go +++ b/internal/cli/inference_test.go @@ -282,10 +282,10 @@ func TestParseOrgOrRepo_OrgMode(t *testing.T) { } func TestParseOrgOrRepo_RepoMode(t *testing.T) { - org, repo, err := parseOrgOrRepo("acme/widget") + org, repo, err := parseOrgOrRepo("acme--corp/widget") require.NoError(t, err) - assert.Equal(t, "acme", org) - assert.Equal(t, "acme/widget", repo) + assert.Equal(t, "acme--corp", org) + assert.Equal(t, "acme--corp/widget", repo) } func TestParseOrgOrRepo_Invalid(t *testing.T) { diff --git a/internal/dispatch/gcf/mintsrc/mintcore/patterns.go.embed b/internal/dispatch/gcf/mintsrc/mintcore/patterns.go.embed index 395861aafb..853142bc0c 100644 --- a/internal/dispatch/gcf/mintsrc/mintcore/patterns.go.embed +++ b/internal/dispatch/gcf/mintsrc/mintcore/patterns.go.embed @@ -11,7 +11,7 @@ import ( // timestamps (exp, iat). Used by both STSVerifier and JWKSVerifier. const maxClockSkew = 30 * time.Second -// GitHubOrgPattern validates GitHub org/user names: alphanumeric or single +// GitHubOrgPattern validates GitHub org/user names: alphanumeric or // hyphens, cannot start or end with a hyphen, max 39 characters. var GitHubOrgPattern = regexp.MustCompile(`^[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,37}[a-zA-Z0-9])?$`) @@ -22,10 +22,9 @@ var RepoNamePattern = regexp.MustCompile(`^[a-zA-Z0-9_.][a-zA-Z0-9._-]{0,99}$`) // RolePattern restricts role to safe lowercase identifiers. var RolePattern = regexp.MustCompile(`^[a-z][a-z0-9_-]*$`) -// ValidateOrgName checks that an org name matches GitHubOrgPattern and -// does not contain double-hyphens (which would be ambiguous in secret names). +// ValidateOrgName checks that an org name matches GitHubOrgPattern. func ValidateOrgName(org string) error { - if !GitHubOrgPattern.MatchString(org) || strings.Contains(org, "--") { + if !GitHubOrgPattern.MatchString(org) { return fmt.Errorf("invalid org name %q", org) } return nil diff --git a/internal/dispatch/gcf/provisioner.go b/internal/dispatch/gcf/provisioner.go index 2770124422..fb451c7ef2 100644 --- a/internal/dispatch/gcf/provisioner.go +++ b/internal/dispatch/gcf/provisioner.go @@ -657,7 +657,7 @@ func (p *Provisioner) Provision(ctx context.Context) (map[string]string, error) } seen := make(map[string]bool) for _, org := range p.cfg.GitHubOrgs { - if !mintcore.GitHubOrgPattern.MatchString(org) || strings.Contains(org, "--") { + if err := mintcore.ValidateOrgName(org); err != nil { return nil, fmt.Errorf("invalid GitHub org name: %q", org) } lower := strings.ToLower(org) @@ -1522,7 +1522,7 @@ func (p *Provisioner) provisionRepoWIFProvider(ctx context.Context) (wifProvider return "", "", fmt.Errorf("repo must be in owner/repo format, got %q", p.cfg.Repo) } partsLower := [2]string{strings.ToLower(parts[0]), strings.ToLower(parts[1])} - if !mintcore.GitHubOrgPattern.MatchString(partsLower[0]) || strings.Contains(partsLower[0], "--") { + if err := mintcore.ValidateOrgName(partsLower[0]); err != nil { return "", "", fmt.Errorf("invalid repo owner %q: must be a valid GitHub org/user name", parts[0]) } if !githubRepoSlugPattern.MatchString(partsLower[1]) { @@ -1577,7 +1577,7 @@ func (p *Provisioner) ProvisionWIF(ctx context.Context) (wifProvider string, err orgs := make([]string, len(p.cfg.GitHubOrgs)) seen := make(map[string]bool) for i, org := range p.cfg.GitHubOrgs { - if !mintcore.GitHubOrgPattern.MatchString(org) || strings.Contains(org, "--") { + if err := mintcore.ValidateOrgName(org); err != nil { return "", fmt.Errorf("invalid GitHub org name: %q", org) } lower := strings.ToLower(org) diff --git a/internal/dispatch/gcf/provisioner_test.go b/internal/dispatch/gcf/provisioner_test.go index 3f56a897bb..4f4e629776 100644 --- a/internal/dispatch/gcf/provisioner_test.go +++ b/internal/dispatch/gcf/provisioner_test.go @@ -250,7 +250,7 @@ func TestProvisioner_Provision_FullFlow(t *testing.T) { State: "ACTIVE", URI: "https://fullsend-mint-abc123.run.app", EnvVars: map[string]string{ - "ALLOWED_ORGS": "test-org", + "ALLOWED_ORGS": "test--org", "ROLE_APP_IDS": `{"coder":"12345"}`, "ALLOWED_ROLES": "coder", "ALLOWED_WORKFLOW_FILES": "*", @@ -259,7 +259,7 @@ func TestProvisioner_Provision_FullFlow(t *testing.T) { p := newTestProvisioner(Config{ ProjectID: "my-project", - GitHubOrgs: []string{"test-org"}, + GitHubOrgs: []string{"test--org"}, AgentPEMs: singleRolePEMs(), AgentAppIDs: singleRoleAppIDs(), FunctionSourceDir: fakeFunctionSourceDir(t), @@ -298,7 +298,7 @@ func TestProvisioner_Provision_FullFlow(t *testing.T) { assert.Equal(t, "my-project", fake.projectIAMBindings[0].ProjectID) assert.Equal(t, "roles/aiplatform.user", fake.projectIAMBindings[0].Role) assert.Contains(t, fake.projectIAMBindings[0].Member, "principalSet://iam.googleapis.com/") - assert.Contains(t, fake.projectIAMBindings[0].Member, "attribute.repository/test-org/.fullsend") + assert.Contains(t, fake.projectIAMBindings[0].Member, "attribute.repository/test--org/.fullsend") // Verify PEMs were zeroed. for role, pem := range p.cfg.AgentPEMs { @@ -1905,7 +1905,7 @@ func TestProvisionWIF_HappyPath(t *testing.T) { fake := newFakeGCFClient() p := NewProvisioner(Config{ ProjectID: "my-project", - GitHubOrgs: []string{"acme"}, + GitHubOrgs: []string{"acme--corp"}, }, fake) wifProvider, err := p.ProvisionWIF(context.Background()) @@ -1918,7 +1918,7 @@ func TestProvisionWIF_HappyPath(t *testing.T) { assert.Contains(t, fake.calls, "CreateWIFProvider") assert.Contains(t, fake.calls, "SetProjectIAMBinding") - assert.Equal(t, "assertion.repository_owner == 'acme'", fake.lastWIFProviderConfig.AttributeCondition) + assert.Equal(t, "assertion.repository_owner == 'acme--corp'", fake.lastWIFProviderConfig.AttributeCondition) } func TestProvisionWIF_MissingProjectID(t *testing.T) { @@ -2076,19 +2076,19 @@ func TestProvisionWIF_RepoScoped(t *testing.T) { fake := newFakeGCFClient() p := NewProvisioner(Config{ ProjectID: "my-project", - GitHubOrgs: []string{"acme"}, - Repo: "acme/widget", + GitHubOrgs: []string{"acme--corp"}, + Repo: "acme--corp/widget", }, fake) wifPath, err := p.ProvisionWIF(context.Background()) require.NoError(t, err) - assert.Equal(t, "gh-acme-widget", fake.lastWIFProviderID) - assert.Equal(t, "assertion.repository == 'acme/widget'", fake.lastWIFProviderConfig.AttributeCondition) - assert.Contains(t, wifPath, "gh-acme-widget") + assert.Equal(t, "gh-acme--corp-widget", fake.lastWIFProviderID) + assert.Equal(t, "assertion.repository == 'acme--corp/widget'", fake.lastWIFProviderConfig.AttributeCondition) + assert.Contains(t, wifPath, "gh-acme--corp-widget") require.Len(t, fake.projectIAMBindings, 1) - assert.Contains(t, fake.projectIAMBindings[0].Member, "attribute.repository/acme/widget") + assert.Contains(t, fake.projectIAMBindings[0].Member, "attribute.repository/acme--corp/widget") assert.NotContains(t, fake.calls, "GetWIFProvider") } @@ -2186,7 +2186,6 @@ func TestProvisionWIF_RepoScoped_RejectsInvalidRepo(t *testing.T) { {"dot as repo", "owner/.", "cannot be"}, {"dotdot as repo", "owner/..", "cannot be"}, {"dot as owner", "./repo", "invalid repo owner"}, - {"double-hyphen in owner", "org--name/repo", "invalid repo owner"}, {"git suffix", "owner/repo.git", "cannot end with"}, } for _, tt := range tests { diff --git a/internal/mintcore/foreign_test.go b/internal/mintcore/foreign_test.go index 6671ca3459..65745b4cae 100644 --- a/internal/mintcore/foreign_test.go +++ b/internal/mintcore/foreign_test.go @@ -25,7 +25,10 @@ func TestValidateTargetOrg(t *testing.T) { if err := validateTargetOrg("halfsend-01"); err != nil { t.Fatalf("valid org: %v", err) } - if err := validateTargetOrg("bad--org"); err == nil { + if err := validateTargetOrg("half--send"); err != nil { + t.Fatalf("valid org with consecutive hyphens: %v", err) + } + if err := validateTargetOrg("-bad-org"); err == nil { t.Fatal("expected invalid org") } } diff --git a/internal/mintcore/patterns.go b/internal/mintcore/patterns.go index 395861aafb..853142bc0c 100644 --- a/internal/mintcore/patterns.go +++ b/internal/mintcore/patterns.go @@ -11,7 +11,7 @@ import ( // timestamps (exp, iat). Used by both STSVerifier and JWKSVerifier. const maxClockSkew = 30 * time.Second -// GitHubOrgPattern validates GitHub org/user names: alphanumeric or single +// GitHubOrgPattern validates GitHub org/user names: alphanumeric or // hyphens, cannot start or end with a hyphen, max 39 characters. var GitHubOrgPattern = regexp.MustCompile(`^[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,37}[a-zA-Z0-9])?$`) @@ -22,10 +22,9 @@ var RepoNamePattern = regexp.MustCompile(`^[a-zA-Z0-9_.][a-zA-Z0-9._-]{0,99}$`) // RolePattern restricts role to safe lowercase identifiers. var RolePattern = regexp.MustCompile(`^[a-z][a-z0-9_-]*$`) -// ValidateOrgName checks that an org name matches GitHubOrgPattern and -// does not contain double-hyphens (which would be ambiguous in secret names). +// ValidateOrgName checks that an org name matches GitHubOrgPattern. func ValidateOrgName(org string) error { - if !GitHubOrgPattern.MatchString(org) || strings.Contains(org, "--") { + if !GitHubOrgPattern.MatchString(org) { return fmt.Errorf("invalid org name %q", org) } return nil