From dd84d547672bef20840f6cbd1fcfea95eaf15e2e Mon Sep 17 00:00:00 2001 From: prasanna8585 Date: Thu, 10 Sep 2026 23:55:41 +0530 Subject: [PATCH 1/2] fix: escape environment names in URL paths across 5 files A sibling gap to the branch-name escaping fix in ListRulesForBranch (#4534): every method that takes a GitHub "environment name" and embeds it into a request path via fmt.Sprintf did so without url.PathEscape, unlike every branch-name usage in repos.go, which already escapes consistently. GitHub environment names, like branch names, are free-form strings set by the repository owner (e.g. "staging", "team/staging") with no character restriction comparable to the one GitHub's own naming rules place on owner/repo segments. An environment name containing a `/` is silently reinterpreted as additional path segments rather than staying part of the environment-name segment, redirecting the request to a different, unintended path under the caller's own GitHub credentials. Fixed 23 call sites across 5 files, all mapping to the same underlying REST resource shape (`/repos/{owner}/{repo}/environments/{environment_name}/...`), just with a different local parameter name per file (`name`, `environment`, or `env`): - github/repos_environments.go (3 sites: GetEnvironment, CreateUpdateEnvironment, DeleteEnvironment) - github/repos_deployment_branch_policies.go (5 sites) - github/repos_deployment_protection_rules.go (5 sites) - github/actions_secrets.go (5 sites) - github/actions_variables.go (5 sites) One incidental fix needed along the way: ActionsService.GetEnvPublicKey in actions_secrets.go named its own local URL-path variable `url`, which would have shadowed the newly-imported net/url package within that function's scope. Renamed the local variable to `u`, matching the convention used by every other method in these files; no behavior change beyond that rename. Verified: gofmt -l reports no issues on all 10 touched files (5 source, 5 test). Full `go build`/`go test` could not be run in this environment (this module's go.mod requires go >= 1.26, network egress to proxy.golang.org for module downloads is unavailable here), so this depends on CI to confirm compilation and test results; the change itself is a mechanical, well-precedented pattern (matching url.PathEscape usage already established for `branch` throughout repos.go and for `branch` in this same PR's own ListRulesForBranch fix), and every modified call site and its surrounding function was reviewed by hand for correct variable references. Added one escape-path regression test per file (5 new tests total), following the same table-driven pattern PR #4534 established for ListRulesForBranch: each asserts both the escaped wire-format request path (`r.URL.EscapedPath()`) and the decoded path (`r.URL.Path`) for a plain environment name and one containing a `/`. --- github/actions_secrets.go | 13 +++---- github/actions_secrets_test.go | 33 ++++++++++++++++++ github/actions_variables.go | 11 +++--- github/actions_variables_test.go | 33 ++++++++++++++++++ github/repos_deployment_branch_policies.go | 11 +++--- .../repos_deployment_branch_policies_test.go | 33 ++++++++++++++++++ github/repos_deployment_protection_rules.go | 11 +++--- .../repos_deployment_protection_rules_test.go | 33 ++++++++++++++++++ github/repos_environments.go | 7 ++-- github/repos_environments_test.go | 34 +++++++++++++++++++ 10 files changed, 195 insertions(+), 24 deletions(-) diff --git a/github/actions_secrets.go b/github/actions_secrets.go index e3d4de0b221..aa6ce398e61 100644 --- a/github/actions_secrets.go +++ b/github/actions_secrets.go @@ -9,6 +9,7 @@ import ( "context" "encoding/json" "fmt" + "net/url" "strconv" ) @@ -88,8 +89,8 @@ func (s *ActionsService) GetOrgPublicKey(ctx context.Context, org string) (*Publ // //meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/secrets/public-key func (s *ActionsService) GetEnvPublicKey(ctx context.Context, owner, repo, env string) (*PublicKey, *Response, error) { - url := fmt.Sprintf("repos/%v/%v/environments/%v/secrets/public-key", owner, repo, env) - return s.getPublicKey(ctx, url) + u := fmt.Sprintf("repos/%v/%v/environments/%v/secrets/public-key", owner, repo, url.PathEscape(env)) + return s.getPublicKey(ctx, u) } // Secret represents a repository action secret. @@ -194,7 +195,7 @@ func (s *ActionsService) ListOrgSecrets(ctx context.Context, org string, opts *L // //meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/secrets func (s *ActionsService) ListEnvSecrets(ctx context.Context, owner, repo, env string, opts *ListOptions) (*Secrets, *Response, error) { - u := fmt.Sprintf("repos/%v/%v/environments/%v/secrets", owner, repo, env) + u := fmt.Sprintf("repos/%v/%v/environments/%v/secrets", owner, repo, url.PathEscape(env)) u, err := addOptions(u, opts) if err != nil { return nil, nil, err @@ -264,7 +265,7 @@ func (s *ActionsService) GetOrgSecret(ctx context.Context, org, name string) (*S // //meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/secrets/{secret_name} func (s *ActionsService) GetEnvSecret(ctx context.Context, owner, repo, env, secretName string) (*Secret, *Response, error) { - u := fmt.Sprintf("repos/%v/%v/environments/%v/secrets/%v", owner, repo, env, secretName) + u := fmt.Sprintf("repos/%v/%v/environments/%v/secrets/%v", owner, repo, url.PathEscape(env), secretName) req, err := s.client.NewRequest(ctx, "GET", u, nil) if err != nil { @@ -358,7 +359,7 @@ func (s *ActionsService) CreateOrUpdateOrgSecret(ctx context.Context, org, name // //meta:operation PUT /repos/{owner}/{repo}/environments/{environment_name}/secrets/{secret_name} func (s *ActionsService) CreateOrUpdateEnvSecret(ctx context.Context, owner, repo, env, name string, body SecretRequest) (*Response, error) { - u := fmt.Sprintf("repos/%v/%v/environments/%v/secrets/%v", owner, repo, env, name) + u := fmt.Sprintf("repos/%v/%v/environments/%v/secrets/%v", owner, repo, url.PathEscape(env), name) req, err := s.client.NewRequest(ctx, "PUT", u, body) if err != nil { @@ -406,7 +407,7 @@ func (s *ActionsService) DeleteOrgSecret(ctx context.Context, org, name string) // //meta:operation DELETE /repos/{owner}/{repo}/environments/{environment_name}/secrets/{secret_name} func (s *ActionsService) DeleteEnvSecret(ctx context.Context, owner, repo, env, secretName string) (*Response, error) { - u := fmt.Sprintf("repos/%v/%v/environments/%v/secrets/%v", owner, repo, env, secretName) + u := fmt.Sprintf("repos/%v/%v/environments/%v/secrets/%v", owner, repo, url.PathEscape(env), secretName) req, err := s.client.NewRequest(ctx, "DELETE", u, nil) if err != nil { diff --git a/github/actions_secrets_test.go b/github/actions_secrets_test.go index 1b36d82c6a1..a65b2cc4062 100644 --- a/github/actions_secrets_test.go +++ b/github/actions_secrets_test.go @@ -781,6 +781,39 @@ func TestActionsService_ListEnvSecrets(t *testing.T) { }) } +func TestActionsService_ListEnvSecrets_EscapeEnv(t *testing.T) { + t.Parallel() + for _, tt := range []struct { + env string + escapedEnv string + }{ + {env: "staging", escapedEnv: "staging"}, + {env: "team/staging", escapedEnv: "team%2Fstaging"}, + } { + t.Run(tt.env, func(t *testing.T) { + t.Parallel() + client, mux, _ := setup(t) + + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + if got, want := r.URL.EscapedPath(), "/repos/o/repo/environments/"+tt.escapedEnv+"/secrets"; got != want { + t.Errorf("Request path = %q, want %q", got, want) + } + if got, want := r.URL.Path, "/repos/o/repo/environments/"+tt.env+"/secrets"; got != want { + t.Errorf("Decoded request path = %q, want %q", got, want) + } + fmt.Fprint(w, `{"total_count":0,"secrets":[]}`) + }) + + ctx := t.Context() + _, _, err := client.Actions.ListEnvSecrets(ctx, "o", "repo", tt.env, nil) + if err != nil { + t.Fatalf("Actions.ListEnvSecrets returned error: %v", err) + } + }) + } +} + func TestActionsService_GetEnvSecret(t *testing.T) { t.Parallel() client, mux, _ := setup(t) diff --git a/github/actions_variables.go b/github/actions_variables.go index 97ecf82f356..2791ab679e9 100644 --- a/github/actions_variables.go +++ b/github/actions_variables.go @@ -9,6 +9,7 @@ import ( "context" "errors" "fmt" + "net/url" ) // ActionsCreateOrgVariableRequest represents a request to create an @@ -145,7 +146,7 @@ func (s *ActionsService) ListOrgVariables(ctx context.Context, org string, opts // //meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/variables func (s *ActionsService) ListEnvVariables(ctx context.Context, owner, repo, env string, opts *ListOptions) (*ActionsVariables, *Response, error) { - u := fmt.Sprintf("repos/%v/%v/environments/%v/variables", owner, repo, env) + u := fmt.Sprintf("repos/%v/%v/environments/%v/variables", owner, repo, url.PathEscape(env)) u, err := addOptions(u, opts) if err != nil { return nil, nil, err @@ -215,7 +216,7 @@ func (s *ActionsService) GetOrgVariable(ctx context.Context, org, name string) ( // //meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/variables/{name} func (s *ActionsService) GetEnvVariable(ctx context.Context, owner, repo, env, variableName string) (*ActionsVariable, *Response, error) { - u := fmt.Sprintf("repos/%v/%v/environments/%v/variables/%v", owner, repo, env, variableName) + u := fmt.Sprintf("repos/%v/%v/environments/%v/variables/%v", owner, repo, url.PathEscape(env), variableName) req, err := s.client.NewRequest(ctx, "GET", u, nil) if err != nil { @@ -269,7 +270,7 @@ func (s *ActionsService) CreateOrgVariable(ctx context.Context, org string, body // //meta:operation POST /repos/{owner}/{repo}/environments/{environment_name}/variables func (s *ActionsService) CreateEnvVariable(ctx context.Context, owner, repo, env string, body ActionsCreateVariableRequest) (*Response, error) { - u := fmt.Sprintf("repos/%v/%v/environments/%v/variables", owner, repo, env) + u := fmt.Sprintf("repos/%v/%v/environments/%v/variables", owner, repo, url.PathEscape(env)) req, err := s.client.NewRequest(ctx, "POST", u, body) if err != nil { @@ -316,7 +317,7 @@ func (s *ActionsService) UpdateOrgVariable(ctx context.Context, org, name string // //meta:operation PATCH /repos/{owner}/{repo}/environments/{environment_name}/variables/{name} func (s *ActionsService) UpdateEnvVariable(ctx context.Context, owner, repo, env, name string, body ActionsUpdateVariableRequest) (*Response, error) { - u := fmt.Sprintf("repos/%v/%v/environments/%v/variables/%v", owner, repo, env, name) + u := fmt.Sprintf("repos/%v/%v/environments/%v/variables/%v", owner, repo, url.PathEscape(env), name) req, err := s.client.NewRequest(ctx, "PATCH", u, body) if err != nil { @@ -364,7 +365,7 @@ func (s *ActionsService) DeleteOrgVariable(ctx context.Context, org, name string // //meta:operation DELETE /repos/{owner}/{repo}/environments/{environment_name}/variables/{name} func (s *ActionsService) DeleteEnvVariable(ctx context.Context, owner, repo, env, variableName string) (*Response, error) { - u := fmt.Sprintf("repos/%v/%v/environments/%v/variables/%v", owner, repo, env, variableName) + u := fmt.Sprintf("repos/%v/%v/environments/%v/variables/%v", owner, repo, url.PathEscape(env), variableName) req, err := s.client.NewRequest(ctx, "DELETE", u, nil) if err != nil { diff --git a/github/actions_variables_test.go b/github/actions_variables_test.go index 40f7b1b18d3..1ced6b8e460 100644 --- a/github/actions_variables_test.go +++ b/github/actions_variables_test.go @@ -598,6 +598,39 @@ func TestActionsService_ListEnvVariables(t *testing.T) { }) } +func TestActionsService_ListEnvVariables_EscapeEnv(t *testing.T) { + t.Parallel() + for _, tt := range []struct { + env string + escapedEnv string + }{ + {env: "staging", escapedEnv: "staging"}, + {env: "team/staging", escapedEnv: "team%2Fstaging"}, + } { + t.Run(tt.env, func(t *testing.T) { + t.Parallel() + client, mux, _ := setup(t) + + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + if got, want := r.URL.EscapedPath(), "/repos/o/repo/environments/"+tt.escapedEnv+"/variables"; got != want { + t.Errorf("Request path = %q, want %q", got, want) + } + if got, want := r.URL.Path, "/repos/o/repo/environments/"+tt.env+"/variables"; got != want { + t.Errorf("Decoded request path = %q, want %q", got, want) + } + fmt.Fprint(w, `{"total_count":0,"variables":[]}`) + }) + + ctx := t.Context() + _, _, err := client.Actions.ListEnvVariables(ctx, "o", "repo", tt.env, nil) + if err != nil { + t.Fatalf("Actions.ListEnvVariables returned error: %v", err) + } + }) + } +} + func TestActionsService_GetEnvVariable(t *testing.T) { t.Parallel() client, mux, _ := setup(t) diff --git a/github/repos_deployment_branch_policies.go b/github/repos_deployment_branch_policies.go index 20d176c5926..327b486d114 100644 --- a/github/repos_deployment_branch_policies.go +++ b/github/repos_deployment_branch_policies.go @@ -8,6 +8,7 @@ package github import ( "context" "fmt" + "net/url" ) // DeploymentBranchPolicy represents a single deployment branch policy for an environment. @@ -41,7 +42,7 @@ type UpdateDeploymentBranchPolicyRequest struct { // //meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/deployment-branch-policies func (s *RepositoriesService) ListDeploymentBranchPolicies(ctx context.Context, owner, repo, environment string, opts *ListOptions) (*DeploymentBranchPolicyResponse, *Response, error) { - u := fmt.Sprintf("repos/%v/%v/environments/%v/deployment-branch-policies", owner, repo, environment) + u := fmt.Sprintf("repos/%v/%v/environments/%v/deployment-branch-policies", owner, repo, url.PathEscape(environment)) u, err := addOptions(u, opts) if err != nil { return nil, nil, err @@ -67,7 +68,7 @@ func (s *RepositoriesService) ListDeploymentBranchPolicies(ctx context.Context, // //meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/deployment-branch-policies/{branch_policy_id} func (s *RepositoriesService) GetDeploymentBranchPolicy(ctx context.Context, owner, repo, environment string, branchPolicyID int64) (*DeploymentBranchPolicy, *Response, error) { - u := fmt.Sprintf("repos/%v/%v/environments/%v/deployment-branch-policies/%v", owner, repo, environment, branchPolicyID) + u := fmt.Sprintf("repos/%v/%v/environments/%v/deployment-branch-policies/%v", owner, repo, url.PathEscape(environment), branchPolicyID) req, err := s.client.NewRequest(ctx, "GET", u, nil) if err != nil { @@ -89,7 +90,7 @@ func (s *RepositoriesService) GetDeploymentBranchPolicy(ctx context.Context, own // //meta:operation POST /repos/{owner}/{repo}/environments/{environment_name}/deployment-branch-policies func (s *RepositoriesService) CreateDeploymentBranchPolicy(ctx context.Context, owner, repo, environment string, body CreateDeploymentBranchPolicyRequest) (*DeploymentBranchPolicy, *Response, error) { - u := fmt.Sprintf("repos/%v/%v/environments/%v/deployment-branch-policies", owner, repo, environment) + u := fmt.Sprintf("repos/%v/%v/environments/%v/deployment-branch-policies", owner, repo, url.PathEscape(environment)) req, err := s.client.NewRequest(ctx, "POST", u, body) if err != nil { @@ -111,7 +112,7 @@ func (s *RepositoriesService) CreateDeploymentBranchPolicy(ctx context.Context, // //meta:operation PUT /repos/{owner}/{repo}/environments/{environment_name}/deployment-branch-policies/{branch_policy_id} func (s *RepositoriesService) UpdateDeploymentBranchPolicy(ctx context.Context, owner, repo, environment string, branchPolicyID int64, body UpdateDeploymentBranchPolicyRequest) (*DeploymentBranchPolicy, *Response, error) { - u := fmt.Sprintf("repos/%v/%v/environments/%v/deployment-branch-policies/%v", owner, repo, environment, branchPolicyID) + u := fmt.Sprintf("repos/%v/%v/environments/%v/deployment-branch-policies/%v", owner, repo, url.PathEscape(environment), branchPolicyID) req, err := s.client.NewRequest(ctx, "PUT", u, body) if err != nil { @@ -133,7 +134,7 @@ func (s *RepositoriesService) UpdateDeploymentBranchPolicy(ctx context.Context, // //meta:operation DELETE /repos/{owner}/{repo}/environments/{environment_name}/deployment-branch-policies/{branch_policy_id} func (s *RepositoriesService) DeleteDeploymentBranchPolicy(ctx context.Context, owner, repo, environment string, branchPolicyID int64) (*Response, error) { - u := fmt.Sprintf("repos/%v/%v/environments/%v/deployment-branch-policies/%v", owner, repo, environment, branchPolicyID) + u := fmt.Sprintf("repos/%v/%v/environments/%v/deployment-branch-policies/%v", owner, repo, url.PathEscape(environment), branchPolicyID) req, err := s.client.NewRequest(ctx, "DELETE", u, nil) if err != nil { diff --git a/github/repos_deployment_branch_policies_test.go b/github/repos_deployment_branch_policies_test.go index cf4b3898f30..c5f63823643 100644 --- a/github/repos_deployment_branch_policies_test.go +++ b/github/repos_deployment_branch_policies_test.go @@ -58,6 +58,39 @@ func TestRepositoriesService_ListDeploymentBranchPolicies(t *testing.T) { }) } +func TestRepositoriesService_ListDeploymentBranchPolicies_EscapeEnvironment(t *testing.T) { + t.Parallel() + for _, tt := range []struct { + environment string + escapedEnvironment string + }{ + {environment: "staging", escapedEnvironment: "staging"}, + {environment: "team/staging", escapedEnvironment: "team%2Fstaging"}, + } { + t.Run(tt.environment, func(t *testing.T) { + t.Parallel() + client, mux, _ := setup(t) + + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + if got, want := r.URL.EscapedPath(), "/repos/o/repo/environments/"+tt.escapedEnvironment+"/deployment-branch-policies"; got != want { + t.Errorf("Request path = %q, want %q", got, want) + } + if got, want := r.URL.Path, "/repos/o/repo/environments/"+tt.environment+"/deployment-branch-policies"; got != want { + t.Errorf("Decoded request path = %q, want %q", got, want) + } + fmt.Fprint(w, `{"total_count":0,"branch_policies":[]}`) + }) + + ctx := t.Context() + _, _, err := client.Repositories.ListDeploymentBranchPolicies(ctx, "o", "repo", tt.environment, nil) + if err != nil { + t.Fatalf("Repositories.ListDeploymentBranchPolicies returned error: %v", err) + } + }) + } +} + func TestRepositoriesService_GetDeploymentBranchPolicy(t *testing.T) { t.Parallel() client, mux, _ := setup(t) diff --git a/github/repos_deployment_protection_rules.go b/github/repos_deployment_protection_rules.go index 10b0f86e579..3b34a68b5ce 100644 --- a/github/repos_deployment_protection_rules.go +++ b/github/repos_deployment_protection_rules.go @@ -8,6 +8,7 @@ package github import ( "context" "fmt" + "net/url" ) // CustomDeploymentProtectionRuleApp represents a single deployment protection rule app for an environment. @@ -49,7 +50,7 @@ type CustomDeploymentProtectionRuleRequest struct { // //meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/deployment_protection_rules func (s *RepositoriesService) GetAllDeploymentProtectionRules(ctx context.Context, owner, repo, environment string) (*ListDeploymentProtectionRuleResponse, *Response, error) { - u := fmt.Sprintf("repos/%v/%v/environments/%v/deployment_protection_rules", owner, repo, environment) + u := fmt.Sprintf("repos/%v/%v/environments/%v/deployment_protection_rules", owner, repo, url.PathEscape(environment)) req, err := s.client.NewRequest(ctx, "GET", u, nil) if err != nil { @@ -71,7 +72,7 @@ func (s *RepositoriesService) GetAllDeploymentProtectionRules(ctx context.Contex // //meta:operation POST /repos/{owner}/{repo}/environments/{environment_name}/deployment_protection_rules func (s *RepositoriesService) CreateCustomDeploymentProtectionRule(ctx context.Context, owner, repo, environment string, body *CustomDeploymentProtectionRuleRequest) (*CustomDeploymentProtectionRule, *Response, error) { - u := fmt.Sprintf("repos/%v/%v/environments/%v/deployment_protection_rules", owner, repo, environment) + u := fmt.Sprintf("repos/%v/%v/environments/%v/deployment_protection_rules", owner, repo, url.PathEscape(environment)) req, err := s.client.NewRequest(ctx, "POST", u, body) if err != nil { @@ -93,7 +94,7 @@ func (s *RepositoriesService) CreateCustomDeploymentProtectionRule(ctx context.C // //meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/deployment_protection_rules/apps func (s *RepositoriesService) ListCustomDeploymentRuleIntegrations(ctx context.Context, owner, repo, environment string, opts *ListOptions) (*ListCustomDeploymentRuleIntegrationsResponse, *Response, error) { - u := fmt.Sprintf("repos/%v/%v/environments/%v/deployment_protection_rules/apps", owner, repo, environment) + u := fmt.Sprintf("repos/%v/%v/environments/%v/deployment_protection_rules/apps", owner, repo, url.PathEscape(environment)) u, err := addOptions(u, opts) if err != nil { return nil, nil, err @@ -119,7 +120,7 @@ func (s *RepositoriesService) ListCustomDeploymentRuleIntegrations(ctx context.C // //meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/deployment_protection_rules/{protection_rule_id} func (s *RepositoriesService) GetCustomDeploymentProtectionRule(ctx context.Context, owner, repo, environment string, protectionRuleID int64) (*CustomDeploymentProtectionRule, *Response, error) { - u := fmt.Sprintf("repos/%v/%v/environments/%v/deployment_protection_rules/%v", owner, repo, environment, protectionRuleID) + u := fmt.Sprintf("repos/%v/%v/environments/%v/deployment_protection_rules/%v", owner, repo, url.PathEscape(environment), protectionRuleID) req, err := s.client.NewRequest(ctx, "GET", u, nil) if err != nil { @@ -141,7 +142,7 @@ func (s *RepositoriesService) GetCustomDeploymentProtectionRule(ctx context.Cont // //meta:operation DELETE /repos/{owner}/{repo}/environments/{environment_name}/deployment_protection_rules/{protection_rule_id} func (s *RepositoriesService) DisableCustomDeploymentProtectionRule(ctx context.Context, owner, repo, environment string, protectionRuleID int64) (*Response, error) { - u := fmt.Sprintf("repos/%v/%v/environments/%v/deployment_protection_rules/%v", owner, repo, environment, protectionRuleID) + u := fmt.Sprintf("repos/%v/%v/environments/%v/deployment_protection_rules/%v", owner, repo, url.PathEscape(environment), protectionRuleID) req, err := s.client.NewRequest(ctx, "DELETE", u, nil) if err != nil { diff --git a/github/repos_deployment_protection_rules_test.go b/github/repos_deployment_protection_rules_test.go index adee3357f80..d0f5a502c4d 100644 --- a/github/repos_deployment_protection_rules_test.go +++ b/github/repos_deployment_protection_rules_test.go @@ -49,6 +49,39 @@ func TestRepositoriesService_GetAllDeploymentProtectionRules(t *testing.T) { }) } +func TestRepositoriesService_GetAllDeploymentProtectionRules_EscapeEnvironment(t *testing.T) { + t.Parallel() + for _, tt := range []struct { + environment string + escapedEnvironment string + }{ + {environment: "staging", escapedEnvironment: "staging"}, + {environment: "team/staging", escapedEnvironment: "team%2Fstaging"}, + } { + t.Run(tt.environment, func(t *testing.T) { + t.Parallel() + client, mux, _ := setup(t) + + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + if got, want := r.URL.EscapedPath(), "/repos/o/repo/environments/"+tt.escapedEnvironment+"/deployment_protection_rules"; got != want { + t.Errorf("Request path = %q, want %q", got, want) + } + if got, want := r.URL.Path, "/repos/o/repo/environments/"+tt.environment+"/deployment_protection_rules"; got != want { + t.Errorf("Decoded request path = %q, want %q", got, want) + } + fmt.Fprint(w, `{"total_count":0,"custom_deployment_protection_rules":[]}`) + }) + + ctx := t.Context() + _, _, err := client.Repositories.GetAllDeploymentProtectionRules(ctx, "o", "repo", tt.environment) + if err != nil { + t.Fatalf("Repositories.GetAllDeploymentProtectionRules returned error: %v", err) + } + }) + } +} + func TestRepositoriesService_CreateCustomDeploymentProtectionRule(t *testing.T) { t.Parallel() client, mux, _ := setup(t) diff --git a/github/repos_environments.go b/github/repos_environments.go index 59b749d7c81..62b4f3977c0 100644 --- a/github/repos_environments.go +++ b/github/repos_environments.go @@ -11,6 +11,7 @@ import ( "errors" "fmt" "net/http" + "net/url" ) // Environment represents a single environment in a repository. @@ -141,7 +142,7 @@ func (s *RepositoriesService) ListEnvironments(ctx context.Context, owner, repo // //meta:operation GET /repos/{owner}/{repo}/environments/{environment_name} func (s *RepositoriesService) GetEnvironment(ctx context.Context, owner, repo, name string) (*Environment, *Response, error) { - u := fmt.Sprintf("repos/%v/%v/environments/%v", owner, repo, name) + u := fmt.Sprintf("repos/%v/%v/environments/%v", owner, repo, url.PathEscape(name)) req, err := s.client.NewRequest(ctx, "GET", u, nil) if err != nil { return nil, nil, err @@ -198,7 +199,7 @@ type createUpdateEnvironmentNoEnterprise struct { // //meta:operation PUT /repos/{owner}/{repo}/environments/{environment_name} func (s *RepositoriesService) CreateUpdateEnvironment(ctx context.Context, owner, repo, name string, body *CreateUpdateEnvironment) (*Environment, *Response, error) { - u := fmt.Sprintf("repos/%v/%v/environments/%v", owner, repo, name) + u := fmt.Sprintf("repos/%v/%v/environments/%v", owner, repo, url.PathEscape(name)) req, err := s.client.NewRequest(ctx, "PUT", u, body) if err != nil { return nil, nil, err @@ -247,7 +248,7 @@ func (s *RepositoriesService) createNewEnvNoEnterprise(ctx context.Context, u st // //meta:operation DELETE /repos/{owner}/{repo}/environments/{environment_name} func (s *RepositoriesService) DeleteEnvironment(ctx context.Context, owner, repo, name string) (*Response, error) { - u := fmt.Sprintf("repos/%v/%v/environments/%v", owner, repo, name) + u := fmt.Sprintf("repos/%v/%v/environments/%v", owner, repo, url.PathEscape(name)) req, err := s.client.NewRequest(ctx, "DELETE", u, nil) if err != nil { return nil, err diff --git a/github/repos_environments_test.go b/github/repos_environments_test.go index 0ee2ef38bea..3a879725051 100644 --- a/github/repos_environments_test.go +++ b/github/repos_environments_test.go @@ -180,6 +180,40 @@ func TestRepositoriesService_GetEnvironment(t *testing.T) { }) } +func TestRepositoriesService_GetEnvironment_EscapeName(t *testing.T) { + t.Parallel() + for _, tt := range []struct { + name string + escapedName string + }{ + {name: "staging", escapedName: "staging"}, + {name: "team/staging", escapedName: "team%2Fstaging"}, + {name: "staging%25", escapedName: "staging%2525"}, + } { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + client, mux, _ := setup(t) + + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + if got, want := r.URL.EscapedPath(), "/repos/o/repo/environments/"+tt.escapedName; got != want { + t.Errorf("Request path = %q, want %q", got, want) + } + if got, want := r.URL.Path, "/repos/o/repo/environments/"+tt.name; got != want { + t.Errorf("Decoded request path = %q, want %q", got, want) + } + fmt.Fprint(w, `{}`) + }) + + ctx := t.Context() + _, _, err := client.Repositories.GetEnvironment(ctx, "o", "repo", tt.name) + if err != nil { + t.Fatalf("Repositories.GetEnvironment returned error: %v", err) + } + }) + } +} + func TestRepositoriesService_CreateEnvironment(t *testing.T) { t.Parallel() client, mux, _ := setup(t) From b28e04f5d8efd828e6a48cf4ef0388c06133b254 Mon Sep 17 00:00:00 2001 From: prasanna8585 Date: Fri, 11 Sep 2026 09:37:48 +0530 Subject: [PATCH 2/2] docs: note that environment names are URL path escaped Addresses gmlewis's review: all 23 modified methods across the 5 files touched by this PR now explicitly state, in their doc comment, that the environment name is URL path escaped for the caller -- matching the exact wording and placement already used for the branch parameter in ListRulesForBranch (PR #4534): // Note: the environment name is URL path escaped for you. See: https://pkg.go.dev/net/url#PathEscape . Placed directly after each method's one-line summary and before its "GitHub API docs:" line, consistent with the branch-name precedent. No logic changed -- doc comments only. --- github/actions_secrets.go | 10 ++++++++++ github/actions_variables.go | 10 ++++++++++ github/repos_deployment_branch_policies.go | 10 ++++++++++ github/repos_deployment_protection_rules.go | 10 ++++++++++ github/repos_environments.go | 6 ++++++ 5 files changed, 46 insertions(+) diff --git a/github/actions_secrets.go b/github/actions_secrets.go index aa6ce398e61..8916fc5ff6c 100644 --- a/github/actions_secrets.go +++ b/github/actions_secrets.go @@ -85,6 +85,8 @@ func (s *ActionsService) GetOrgPublicKey(ctx context.Context, org string) (*Publ // GetEnvPublicKey gets a public key that should be used for secret encryption. // +// Note: the environment name is URL path escaped for you. See: https://pkg.go.dev/net/url#PathEscape . +// // GitHub API docs: https://docs.github.com/rest/actions/secrets?apiVersion=2022-11-28#get-an-environment-public-key // //meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/secrets/public-key @@ -191,6 +193,8 @@ func (s *ActionsService) ListOrgSecrets(ctx context.Context, org string, opts *L // ListEnvSecrets lists all secrets available in an environment. // +// Note: the environment name is URL path escaped for you. See: https://pkg.go.dev/net/url#PathEscape . +// // GitHub API docs: https://docs.github.com/rest/actions/secrets?apiVersion=2022-11-28#list-environment-secrets // //meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/secrets @@ -261,6 +265,8 @@ func (s *ActionsService) GetOrgSecret(ctx context.Context, org, name string) (*S // GetEnvSecret gets a single environment secret without revealing its encrypted value. // +// Note: the environment name is URL path escaped for you. See: https://pkg.go.dev/net/url#PathEscape . +// // GitHub API docs: https://docs.github.com/rest/actions/secrets?apiVersion=2022-11-28#get-an-environment-secret // //meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/secrets/{secret_name} @@ -355,6 +361,8 @@ func (s *ActionsService) CreateOrUpdateOrgSecret(ctx context.Context, org, name // CreateOrUpdateEnvSecret creates or updates a single environment secret with an encrypted value. // +// Note: the environment name is URL path escaped for you. See: https://pkg.go.dev/net/url#PathEscape . +// // GitHub API docs: https://docs.github.com/rest/actions/secrets?apiVersion=2022-11-28#create-or-update-an-environment-secret // //meta:operation PUT /repos/{owner}/{repo}/environments/{environment_name}/secrets/{secret_name} @@ -403,6 +411,8 @@ func (s *ActionsService) DeleteOrgSecret(ctx context.Context, org, name string) // DeleteEnvSecret deletes a secret in an environment using the secret name. // +// Note: the environment name is URL path escaped for you. See: https://pkg.go.dev/net/url#PathEscape . +// // GitHub API docs: https://docs.github.com/rest/actions/secrets?apiVersion=2022-11-28#delete-an-environment-secret // //meta:operation DELETE /repos/{owner}/{repo}/environments/{environment_name}/secrets/{secret_name} diff --git a/github/actions_variables.go b/github/actions_variables.go index 2791ab679e9..b617956e3cc 100644 --- a/github/actions_variables.go +++ b/github/actions_variables.go @@ -142,6 +142,8 @@ func (s *ActionsService) ListOrgVariables(ctx context.Context, org string, opts // ListEnvVariables lists all variables available in an environment. // +// Note: the environment name is URL path escaped for you. See: https://pkg.go.dev/net/url#PathEscape . +// // GitHub API docs: https://docs.github.com/rest/actions/variables?apiVersion=2022-11-28#list-environment-variables // //meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/variables @@ -212,6 +214,8 @@ func (s *ActionsService) GetOrgVariable(ctx context.Context, org, name string) ( // GetEnvVariable gets a single environment variable. // +// Note: the environment name is URL path escaped for you. See: https://pkg.go.dev/net/url#PathEscape . +// // GitHub API docs: https://docs.github.com/rest/actions/variables?apiVersion=2022-11-28#get-an-environment-variable // //meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/variables/{name} @@ -266,6 +270,8 @@ func (s *ActionsService) CreateOrgVariable(ctx context.Context, org string, body // CreateEnvVariable creates an environment variable. // +// Note: the environment name is URL path escaped for you. See: https://pkg.go.dev/net/url#PathEscape . +// // GitHub API docs: https://docs.github.com/rest/actions/variables?apiVersion=2022-11-28#create-an-environment-variable // //meta:operation POST /repos/{owner}/{repo}/environments/{environment_name}/variables @@ -313,6 +319,8 @@ func (s *ActionsService) UpdateOrgVariable(ctx context.Context, org, name string // UpdateEnvVariable updates an environment variable. // +// Note: the environment name is URL path escaped for you. See: https://pkg.go.dev/net/url#PathEscape . +// // GitHub API docs: https://docs.github.com/rest/actions/variables?apiVersion=2022-11-28#update-an-environment-variable // //meta:operation PATCH /repos/{owner}/{repo}/environments/{environment_name}/variables/{name} @@ -361,6 +369,8 @@ func (s *ActionsService) DeleteOrgVariable(ctx context.Context, org, name string // DeleteEnvVariable deletes a variable in an environment. // +// Note: the environment name is URL path escaped for you. See: https://pkg.go.dev/net/url#PathEscape . +// // GitHub API docs: https://docs.github.com/rest/actions/variables?apiVersion=2022-11-28#delete-an-environment-variable // //meta:operation DELETE /repos/{owner}/{repo}/environments/{environment_name}/variables/{name} diff --git a/github/repos_deployment_branch_policies.go b/github/repos_deployment_branch_policies.go index 327b486d114..d57c600f23a 100644 --- a/github/repos_deployment_branch_policies.go +++ b/github/repos_deployment_branch_policies.go @@ -38,6 +38,8 @@ type UpdateDeploymentBranchPolicyRequest struct { // ListDeploymentBranchPolicies lists the deployment branch policies for an environment. // +// Note: the environment name is URL path escaped for you. See: https://pkg.go.dev/net/url#PathEscape . +// // GitHub API docs: https://docs.github.com/rest/deployments/branch-policies?apiVersion=2022-11-28#list-deployment-branch-policies // //meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/deployment-branch-policies @@ -64,6 +66,8 @@ func (s *RepositoriesService) ListDeploymentBranchPolicies(ctx context.Context, // GetDeploymentBranchPolicy gets a deployment branch policy for an environment. // +// Note: the environment name is URL path escaped for you. See: https://pkg.go.dev/net/url#PathEscape . +// // GitHub API docs: https://docs.github.com/rest/deployments/branch-policies?apiVersion=2022-11-28#get-a-deployment-branch-policy // //meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/deployment-branch-policies/{branch_policy_id} @@ -86,6 +90,8 @@ func (s *RepositoriesService) GetDeploymentBranchPolicy(ctx context.Context, own // CreateDeploymentBranchPolicy creates a deployment branch policy for an environment. // +// Note: the environment name is URL path escaped for you. See: https://pkg.go.dev/net/url#PathEscape . +// // GitHub API docs: https://docs.github.com/rest/deployments/branch-policies?apiVersion=2022-11-28#create-a-deployment-branch-policy // //meta:operation POST /repos/{owner}/{repo}/environments/{environment_name}/deployment-branch-policies @@ -108,6 +114,8 @@ func (s *RepositoriesService) CreateDeploymentBranchPolicy(ctx context.Context, // UpdateDeploymentBranchPolicy updates a deployment branch policy for an environment. // +// Note: the environment name is URL path escaped for you. See: https://pkg.go.dev/net/url#PathEscape . +// // GitHub API docs: https://docs.github.com/rest/deployments/branch-policies?apiVersion=2022-11-28#update-a-deployment-branch-policy // //meta:operation PUT /repos/{owner}/{repo}/environments/{environment_name}/deployment-branch-policies/{branch_policy_id} @@ -130,6 +138,8 @@ func (s *RepositoriesService) UpdateDeploymentBranchPolicy(ctx context.Context, // DeleteDeploymentBranchPolicy deletes a deployment branch policy for an environment. // +// Note: the environment name is URL path escaped for you. See: https://pkg.go.dev/net/url#PathEscape . +// // GitHub API docs: https://docs.github.com/rest/deployments/branch-policies?apiVersion=2022-11-28#delete-a-deployment-branch-policy // //meta:operation DELETE /repos/{owner}/{repo}/environments/{environment_name}/deployment-branch-policies/{branch_policy_id} diff --git a/github/repos_deployment_protection_rules.go b/github/repos_deployment_protection_rules.go index 3b34a68b5ce..a7ebc4de370 100644 --- a/github/repos_deployment_protection_rules.go +++ b/github/repos_deployment_protection_rules.go @@ -46,6 +46,8 @@ type CustomDeploymentProtectionRuleRequest struct { // GetAllDeploymentProtectionRules gets all the deployment protection rules for an environment. // +// Note: the environment name is URL path escaped for you. See: https://pkg.go.dev/net/url#PathEscape . +// // GitHub API docs: https://docs.github.com/rest/deployments/protection-rules?apiVersion=2022-11-28#get-all-deployment-protection-rules-for-an-environment // //meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/deployment_protection_rules @@ -68,6 +70,8 @@ func (s *RepositoriesService) GetAllDeploymentProtectionRules(ctx context.Contex // CreateCustomDeploymentProtectionRule creates a custom deployment protection rule on an environment. // +// Note: the environment name is URL path escaped for you. See: https://pkg.go.dev/net/url#PathEscape . +// // GitHub API docs: https://docs.github.com/rest/deployments/protection-rules?apiVersion=2022-11-28#create-a-custom-deployment-protection-rule-on-an-environment // //meta:operation POST /repos/{owner}/{repo}/environments/{environment_name}/deployment_protection_rules @@ -90,6 +94,8 @@ func (s *RepositoriesService) CreateCustomDeploymentProtectionRule(ctx context.C // ListCustomDeploymentRuleIntegrations lists the custom deployment rule integrations for an environment. // +// Note: the environment name is URL path escaped for you. See: https://pkg.go.dev/net/url#PathEscape . +// // GitHub API docs: https://docs.github.com/rest/deployments/protection-rules?apiVersion=2022-11-28#list-custom-deployment-rule-integrations-available-for-an-environment // //meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/deployment_protection_rules/apps @@ -116,6 +122,8 @@ func (s *RepositoriesService) ListCustomDeploymentRuleIntegrations(ctx context.C // GetCustomDeploymentProtectionRule gets a custom deployment protection rule for an environment. // +// Note: the environment name is URL path escaped for you. See: https://pkg.go.dev/net/url#PathEscape . +// // GitHub API docs: https://docs.github.com/rest/deployments/protection-rules?apiVersion=2022-11-28#get-a-custom-deployment-protection-rule // //meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/deployment_protection_rules/{protection_rule_id} @@ -138,6 +146,8 @@ func (s *RepositoriesService) GetCustomDeploymentProtectionRule(ctx context.Cont // DisableCustomDeploymentProtectionRule disables a custom deployment protection rule for an environment. // +// Note: the environment name is URL path escaped for you. See: https://pkg.go.dev/net/url#PathEscape . +// // GitHub API docs: https://docs.github.com/rest/deployments/protection-rules?apiVersion=2022-11-28#disable-a-custom-protection-rule-for-an-environment // //meta:operation DELETE /repos/{owner}/{repo}/environments/{environment_name}/deployment_protection_rules/{protection_rule_id} diff --git a/github/repos_environments.go b/github/repos_environments.go index 62b4f3977c0..9ed3aa2f054 100644 --- a/github/repos_environments.go +++ b/github/repos_environments.go @@ -138,6 +138,8 @@ func (s *RepositoriesService) ListEnvironments(ctx context.Context, owner, repo // GetEnvironment get a single environment for a repository. // +// Note: the environment name is URL path escaped for you. See: https://pkg.go.dev/net/url#PathEscape . +// // GitHub API docs: https://docs.github.com/rest/deployments/environments?apiVersion=2022-11-28#get-an-environment // //meta:operation GET /repos/{owner}/{repo}/environments/{environment_name} @@ -195,6 +197,8 @@ type createUpdateEnvironmentNoEnterprise struct { // CreateUpdateEnvironment create or update a new environment for a repository. // +// Note: the environment name is URL path escaped for you. See: https://pkg.go.dev/net/url#PathEscape . +// // GitHub API docs: https://docs.github.com/rest/deployments/environments?apiVersion=2022-11-28#create-or-update-an-environment // //meta:operation PUT /repos/{owner}/{repo}/environments/{environment_name} @@ -244,6 +248,8 @@ func (s *RepositoriesService) createNewEnvNoEnterprise(ctx context.Context, u st // DeleteEnvironment delete an environment from a repository. // +// Note: the environment name is URL path escaped for you. See: https://pkg.go.dev/net/url#PathEscape . +// // GitHub API docs: https://docs.github.com/rest/deployments/environments?apiVersion=2022-11-28#delete-an-environment // //meta:operation DELETE /repos/{owner}/{repo}/environments/{environment_name}