Input validation / Credential handling (GitHub App GHES provisioning)

Proof-of-concept (actionable steps to illustrate the potential vulnerability path before the fix): Background: Prior to this patch, the provisioning code path for GitHub App credentials did not strictly validate certain fields. In particular, appID could be non-numeric and clientSecret was not explicitly forbidden, which could allow misconfiguration or leakage of credentials during provisioning. PoC scenario (pre-fix behavior): 1) Prepare a Grafana provisioning manifest that configures a GHES GitHub Enterprise connection (type: githubEnterprise). 2) Supply a non-numeric appID and include a clientSecret alongside a valid privateKey in the provisioning secret store. 3) Apply the provisioning manifest against Grafana. YAML example (illustrative): --- spec: type: "githubEnterprise" githubEnterprise: appID: "not-a-number" # non-numeric value (should be numeric) installationID: "12345" serverUrl: "https://ghes.example.com" secure: privateKey: "BASE64_ENCODED_PRIVATE_KEY" clientSecret: "sensitive-client-secret" # sensitive secret that may be logged or stored insecurely if not properly forbidden Expected (pre-fix) behavior: - The system may proceed with provisioning without rejecting non-numeric appID. - clientSecret could be accepted/used by downstream processes, potentially leaking or misusing credentials during token exchange with GHES. - No explicit validation error is raised for appID contents or clientSecret in this path, enabling misconfiguration to slip through. Post-fix expectation (what the patch enforces): - appID must be numeric; provisioning errors out if not (strconv.Atoi check). - clientSecret is forbidden in the GitHub provisioning path; an error is raised if provided. - privateKey must be provided; provisioning errors out if missing. This PoC is intended to illustrate how the vulnerability path could be exercised prior to the fix. In a real environment you should not expose or reuse secrets in public repositories or logs.

diff --git a/apps/provisioning/kinds/connection.cue b/apps/provisioning/kinds/connection.cue index cee9713c24d76..de78ab87c41ff 100644 --- a/apps/provisioning/kinds/connection.cue +++ b/apps/provisioning/kinds/connection.cue @@ -26,6 +26,18 @@ connection: { // GitHub App installation ID installationID: int } + #GitHubEnterpriseConnectionConfig: { + // App-level information + // GitHub App ID + appID: int + + // Installation-level information + // GitHub App installation ID + installationID: int + + // The GitHub Enterprise Server URL (e.g. `https://ghes.example.com`). + serverUrl: string + } #BitbucketConnectionConfig: { // The app clientID clientID: string @@ -45,12 +57,15 @@ connection: { } spec: { // The connection provider type - type: "github" | "bitbucket" | "gitlab" - // The connection URL + type: "github" | "githubEnterprise" | "bitbucket" | "gitlab" + // The connection URL. url: *"" | string - // GitHub connection configuration - // Only applicable when provider is "github" + // GitHub connection configuration. + // Only applicable when provider is "github". github?: #GitHubConnectionConfig + // GitHub Enterprise Server connection configuration. + // Only applicable when provider is "githubEnterprise". + githubEnterprise?: #GitHubEnterpriseConnectionConfig // Bitbucket connection configuration // Only applicable when provider is "bitbucket" bitbucket?: #BitbucketConnectionConfig diff --git a/apps/provisioning/kinds/repository.cue b/apps/provisioning/kinds/repository.cue index cd8b3ee3cb72f..343a59456b9e8 100644 --- a/apps/provisioning/kinds/repository.cue +++ b/apps/provisioning/kinds/repository.cue @@ -36,6 +36,22 @@ repository: { // Path is the subdirectory for the Grafana data. If specified, Grafana will ignore anything that is outside this directory in the repository. path?: string } + #GitHubEnterpriseRepositoryConfig: { + // The GitHub Enterprise Server URL (e.g. `https://ghes.example.com`). + serverUrl?: string + // The repository URL on the GHES server (e.g. `https://ghes.example.com/example/test`). + url?: string + // The branch to use in the repository. + branch: string + // Token for accessing the repository. If set, it will be encrypted into encryptedToken, then set to an empty string again. + token?: string + // Token for accessing the repository, but encrypted. This is not possible to read back to a user decrypted. + encryptedToken?: [...string] + // Whether we should show dashboard previews for pull requests. + generateDashboardPreviews?: bool + // Path is the subdirectory for the Grafana data inside the repository. + path?: string + } #GitRepositoryConfig: { // The repository URL (e.g. `https://github.com/example/test.git`). url?: string @@ -156,7 +172,7 @@ repository: { // Sync settings -- how values are pulled from the repository into grafana sync: #SyncOptions // The repository type. When selected oneOf the values below should be non-nil - type: "local" | "github" | "git" | "bitbucket" | "gitlab" + type: "local" | "github" | "githubEnterprise" | "git" | "bitbucket" | "gitlab" // Webhook settings for the repository. webhook?: #WebhookConfig // The repository on the local file system. @@ -165,6 +181,9 @@ repository: { // The repository on GitHub. // Mutually exclusive with local | github | git. github?: #GitHubRepositoryConfig + // The repository on a self-managed GitHub Enterprise Server (GHES). + // Mutually exclusive with the other repository configs. + githubEnterprise?: #GitHubEnterpriseRepositoryConfig // The repository on Git. // Mutually exclusive with local | github | git. git?: #GitRepositoryConfig diff --git a/apps/provisioning/pkg/apis/provisioning/v0alpha1/connections.go b/apps/provisioning/pkg/apis/provisioning/v0alpha1/connections.go index 8f5cf773589c7..d162510e8f74e 100644 --- a/apps/provisioning/pkg/apis/provisioning/v0alpha1/connections.go +++ b/apps/provisioning/pkg/apis/provisioning/v0alpha1/connections.go @@ -59,6 +59,23 @@ func (GitHubConnectionConfig) OpenAPIModelName() string { return OpenAPIPrefix + "GitHubConnectionConfig" } +// GitHubEnterpriseConnectionConfig describes a GitHub App installation against a +// self-managed GitHub Enterprise Server (GHES) instance. +type GitHubEnterpriseConnectionConfig struct { + // GitHub App ID + AppID string `json:"appID"` + + // GitHub App installation ID + InstallationID string `json:"installationID"` + + // The GitHub Enterprise Server URL (e.g. `https://ghes.example.com`). + ServerURL string `json:"serverUrl"` +} + +func (GitHubEnterpriseConnectionConfig) OpenAPIModelName() string { + return OpenAPIPrefix + "GitHubEnterpriseConnectionConfig" +} + type BitbucketConnectionConfig struct { // App client ID ClientID string `json:"clientID"` @@ -87,9 +104,10 @@ func (ConnectionType) OpenAPIModelName() string { // ConnectionType values. const ( - GithubConnectionType ConnectionType = "github" - GitlabConnectionType ConnectionType = "gitlab" - BitbucketConnectionType ConnectionType = "bitbucket" + GithubConnectionType ConnectionType = "github" + GithubEnterpriseConnectionType ConnectionType = "githubEnterprise" + GitlabConnectionType ConnectionType = "gitlab" + BitbucketConnectionType ConnectionType = "bitbucket" ) type ConnectionSpec struct { @@ -105,6 +123,9 @@ type ConnectionSpec struct { // GitHub connection configuration // Only applicable when provider is "github" GitHub *GitHubConnectionConfig `json:"github,omitempty"` + // GitHub Enterprise Server connection configuration + // Only applicable when provider is "githubEnterprise" + GitHubEnterprise *GitHubEnterpriseConnectionConfig `json:"githubEnterprise,omitempty"` // Bitbucket connection configuration // Only applicable when provider is "bitbucket" Bitbucket *BitbucketConnectionConfig `json:"bitbucket,omitempty"` diff --git a/apps/provisioning/pkg/apis/provisioning/v0alpha1/register.go b/apps/provisioning/pkg/apis/provisioning/v0alpha1/register.go index c1785a048d11c..3e48d9b7b01e4 100644 --- a/apps/provisioning/pkg/apis/provisioning/v0alpha1/register.go +++ b/apps/provisioning/pkg/apis/provisioning/v0alpha1/register.go @@ -41,6 +41,10 @@ var RepositoryResourceInfo = utils.NewResourceInfo(GROUP, VERSION, target = m.Spec.Local.Path case GitHubRepositoryType: target = m.Spec.GitHub.URL + case GitHubEnterpriseRepositoryType: + if m.Spec.GitHubEnterprise != nil { + target = m.Spec.GitHubEnterprise.URL + } case GitRepositoryType: target = m.Spec.Git.URL case BitbucketRepositoryType: @@ -139,6 +143,11 @@ var ConnectionResourceInfo = utils.NewResourceInfo(GROUP, VERSION, case GithubConnectionType: appID = m.Spec.GitHub.AppID installationID = m.Spec.GitHub.InstallationID + case GithubEnterpriseConnectionType: + if m.Spec.GitHubEnterprise != nil { + appID = m.Spec.GitHubEnterprise.AppID + installationID = m.Spec.GitHubEnterprise.InstallationID + } case BitbucketConnectionType: clientID = m.Spec.Bitbucket.ClientID case GitlabConnectionType: diff --git a/apps/provisioning/pkg/apis/provisioning/v0alpha1/types.go b/apps/provisioning/pkg/apis/provisioning/v0alpha1/types.go index 520d0eda344f1..56ae6ef2a5e45 100644 --- a/apps/provisioning/pkg/apis/provisioning/v0alpha1/types.go +++ b/apps/provisioning/pkg/apis/provisioning/v0alpha1/types.go @@ -86,6 +86,29 @@ func (GitHubRepositoryConfig) OpenAPIModelName() string { return OpenAPIPrefix + "GitHubRepositoryConfig" } +// GitHubEnterpriseRepositoryConfig describes a repository hosted on a self-managed +// GitHub Enterprise Server (GHES) instance. +type GitHubEnterpriseRepositoryConfig struct { + // The GitHub Enterprise Server URL (e.g. `https://ghes.example.com`). + ServerURL string `json:"serverUrl,omitempty"` + + // The repository URL on the GHES server (e.g. `https://ghes.example.com/example/test`). + URL string `json:"url,omitempty"` + + // The branch to use in the repository. + Branch string `json:"branch"` + + // Whether we should show dashboard previews for pull requests. + GenerateDashboardPreviews bool `json:"generateDashboardPreviews,omitempty"` + + // Path is the subdirectory for the Grafana data inside the repository. + Path string `json:"path,omitempty"` +} + +func (GitHubEnterpriseRepositoryConfig) OpenAPIModelName() string { + return OpenAPIPrefix + "GitHubEnterpriseRepositoryConfig" +} + type GitRepositoryConfig struct { // The repository URL (e.g. `https://github.com/example/test`). URL string `json:"url,omitempty"` @@ -149,18 +172,23 @@ func (RepositoryType) OpenAPIModelName() string { return OpenAPIPrefix + "RepositoryType" } +func (r RepositoryType) String() string { + return string(r) +} + // RepositoryType values const ( - LocalRepositoryType RepositoryType = "local" - GitHubRepositoryType RepositoryType = "github" - GitRepositoryType RepositoryType = "git" - BitbucketRepositoryType RepositoryType = "bitbucket" - GitLabRepositoryType RepositoryType = "gitlab" + LocalRepositoryType RepositoryType = "local" + GitHubRepositoryType RepositoryType = "github" + GitHubEnterpriseRepositoryType RepositoryType = "githubEnterprise" + GitRepositoryType RepositoryType = "git" + BitbucketRepositoryType RepositoryType = "bitbucket" + GitLabRepositoryType RepositoryType = "gitlab" ) // IsGit returns true if the repository type is git or github func (r RepositoryType) IsGit() bool { - return r == GitRepositoryType || r == GitHubRepositoryType || r == BitbucketRepositoryType || r == GitLabRepositoryType + return r == GitRepositoryType || r == GitHubRepositoryType || r == GitHubEnterpriseRepositoryType || r == BitbucketRepositoryType || r == GitLabRepositoryType } // Branch returns the branch for git-based repositories @@ -175,6 +203,10 @@ func (r *Repository) Branch() string { if r.Spec.GitHub != nil { return r.Spec.GitHub.Branch } + case GitHubEnterpriseRepositoryType: + if r.Spec.GitHubEnterprise != nil { + return r.Spec.GitHubEnterprise.Branch + } case GitRepositoryType: if r.Spec.Git != nil { return r.Spec.Git.Branch @@ -206,6 +238,10 @@ func (r *Repository) URL() string { if r.Spec.GitHub != nil { return r.Spec.GitHub.URL } + case GitHubEnterpriseRepositoryType: + if r.Spec.GitHubEnterprise != nil { + return r.Spec.GitHubEnterprise.URL + } case GitRepositoryType: if r.Spec.Git != nil { return r.Spec.Git.URL @@ -231,6 +267,10 @@ func (r *Repository) Path() string { if r.Spec.GitHub != nil { return r.Spec.GitHub.Path } + case GitHubEnterpriseRepositoryType: + if r.Spec.GitHubEnterprise != nil { + return r.Spec.GitHubEnterprise.Path + } case GitRepositoryType: if r.Spec.Git != nil { return r.Spec.Git.Path @@ -319,6 +359,10 @@ type RepositorySpec struct { // Mutually exclusive with local | github | git. GitHub *GitHubRepositoryConfig `json:"github,omitempty"` + // The repository on a self-managed GitHub Enterprise Server (GHES). + // Mutually exclusive with local | github | git. + GitHubEnterprise *GitHubEnterpriseRepositoryConfig `json:"githubEnterprise,omitempty"` + // The repository on Git. // Mutually exclusive with local | github | git. Git *GitRepositoryConfig `json:"git,omitempty"` diff --git a/apps/provisioning/pkg/connection/github/validator.go b/apps/provisioning/pkg/connection/github/validator.go index 3702179d83b9e..b7097fe696b42 100644 --- a/apps/provisioning/pkg/connection/github/validator.go +++ b/apps/provisioning/pkg/connection/github/validator.go @@ -3,6 +3,7 @@ package github import ( "context" "encoding/base64" + "fmt" "strconv" "github.com/golang-jwt/jwt/v4" @@ -36,12 +37,24 @@ func Validate(_ context.Context, obj runtime.Object) field.ErrorList { return list } + list = append(list, ValidateGitHubAppCredentials(conn, "GitHub", conn.Spec.GitHub.AppID, conn.Spec.GitHub.InstallationID, field.NewPath("spec", "github"))...) + return list +} + +// ValidateGitHubAppCredentials performs structural validation of the GitHub App credential +// fields shared by github and githubEnterprise connections. label is interpolated into +// error messages (e.g. "GitHub", "GitHub Enterprise") so the source of the violation is +// clear. basePath is the field path of the spec section holding the credentials +// (e.g. spec.github or spec.githubEnterprise). +func ValidateGitHubAppCredentials(conn *provisioning.Connection, label, appID, installationID string, basePath *field.Path) field.ErrorList { + var list field.ErrorList + // Check if required secure values are present (without decryption) if conn.Secure.PrivateKey.IsZero() { - list = append(list, field.Required(field.NewPath("secure", "privateKey"), "privateKey must be specified for GitHub connection")) + list = append(list, field.Required(field.NewPath("secure", "privateKey"), fmt.Sprintf("privateKey must be specified for %s connection", label))) } if !conn.Secure.ClientSecret.IsZero() { - list = append(list, field.Forbidden(field.NewPath("secure", "clientSecret"), "clientSecret is forbidden in GitHub connection")) + list = append(list, field.Forbidden(field.NewPath("secure", "clientSecret"), fmt.Sprintf("clientSecret is forbidden in %s connection", label))) } // Validate private key content if new is provided @@ -59,22 +72,17 @@ func Validate(_ context.Context, obj runtime.Object) field.ErrorList { } } - // Validate the existence of GitHub configuration fields - if conn.Spec.GitHub.AppID == "" { - list = append(list, field.Required(field.NewPath("spec", "github", "appID"), "appID must be specified for GitHub connection")) - } - if conn.Spec.GitHub.InstallationID == "" { - list = append(list, field.Required(field.NewPath("spec", "github", "installationID"), "installationID must be specified for GitHub connection")) - } - - // Validating the correctness of Github config fields - _, err := strconv.Atoi(conn.Spec.GitHub.AppID) - if err != nil { - list = append(list, field.Invalid(field.NewPath("spec", "github", "appID"), conn.Spec.GitHub.AppID, "appID must be a numeric value")) + // Validate the existence and correctness of GitHub configuration fields. + // Skip the numeric check on empty values to avoid emitting both Required and Invalid for the same field. + if appID == "" { + list = append(list, field.Required(basePath.Child("appID"), fmt.Sprintf("appID must be specified for %s connection", label))) + } else if _, err := strconv.Atoi(appID); err != nil { + list = append(list, field.Invalid(basePath.Child("appID"), appID, "appID must be a numeric value")) } - _, err = s ... [truncated]