Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Code Ownership & Review Assignment Tool - GitHub CODEOWNERS but better
- [Getting Started](#getting-started)
- [GitHub Configuration](#github-configuration)
- [GitHub Teams Support](#github-teams-support)
- [GitHub Enterprise](#github-enterprise)
- [Configuration](#configuration)
- [.codeowners File Spec](#codeowners-file-spec)
- [Advanced Configuration](#advanced-configuration)
Expand Down Expand Up @@ -112,6 +113,16 @@ It is recommended to also set up a rerun workflow on `pull_request_review` to re

If you plan to have organization teams as code owners, you will need to use a PAT that has organization [read access for Members and Administration](https://docs.github.com/en/rest/authentication/permissions-required-for-fine-grained-personal-access-tokens) as the token. If you do not have organization teams as owners, [GITHUB_TOKEN](https://docs.github.com/en/actions/security-for-github-actions/security-guides/automatic-token-authentication#using-the-github_token-in-a-workflow) should be sufficient.

### GitHub Enterprise

To use a GitHub Enterprise instance instead of the public GitHub API, set the `github-api-url` input:

```yaml
github-api-url: 'https://ghe.example.com'
```

The `/api/v3` suffix is optional. For GitHub Enterprise Cloud with data residency, use `https://api.<tenant>.ghe.com`. If the input is not set, the public GitHub API is used.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small correction: the suffix is only optional for some hostnames — go-github skips appending /api/v3 when the host starts with api., so e.g. a GHES instance at api.corp.example.com would break without it. Simplest guidance that's always right:

Suggested change
The `/api/v3` suffix is optional. For GitHub Enterprise Cloud with data residency, use `https://api.<tenant>.ghe.com`. If the input is not set, the public GitHub API is used.
The value should be the instance's exact API URL — the same value as the `github.api_url` context (`GITHUB_API_URL`). For GitHub Enterprise Server this includes the `/api/v3` suffix; for GitHub Enterprise Cloud with data residency use `https://api.<tenant>.ghe.com`. If the input is not set, it defaults to the API URL of the instance running the workflow.


## Configuration

### .codeowners File Spec
Expand Down
5 changes: 5 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ inputs:
description: 'The owner and repository name. For example `octocat/Hello-World`'
required: true
default: '${{ github.repository }}'
github-api-url:
description: 'GitHub API base URL. Set for GitHub Enterprise, e.g. https://ghe.example.com/api/v3. Defaults to the public GitHub API.'
required: false
default: ''
Comment on lines +19 to +21

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The runner always exports the instance's API URL, so defaulting to it makes GHES work with zero config — and avoids the footgun where someone forgets this input and the action silently talks to public api.github.com with their enterprise token. (A getEnv fallback in main.go wouldn't catch it, since the run step always exports INPUT_GITHUB-API-URL, even when empty.)

Suggested change
description: 'GitHub API base URL. Set for GitHub Enterprise, e.g. https://ghe.example.com/api/v3. Defaults to the public GitHub API.'
required: false
default: ''
description: 'GitHub API base URL. Defaults to the API URL of the instance running the workflow. If overriding, use the exact value of the `github.api_url` context (GitHub Enterprise Server includes the /api/v3 suffix; GHE Cloud with data residency does not).'
required: false
default: '${{ github.api_url }}'

verbose:
description: 'Print debug info'
required: false
Expand Down Expand Up @@ -111,6 +115,7 @@ runs:
INPUT_GITHUB-TOKEN: ${{ inputs.github-token }}
INPUT_PR: ${{ inputs.pr }}
INPUT_REPOSITORY: ${{ inputs.repository }}
INPUT_GITHUB-API-URL: ${{ inputs.github-api-url }}
INPUT_VERBOSE: ${{ inputs.verbose }}
INPUT_QUIET: ${{ inputs.quiet }}
BIN: ${{ steps.resolve.outputs.bin }}
Expand Down
3 changes: 2 additions & 1 deletion internal/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ func (od *OutputData) UpdateOutputData(success bool, message string, stillRequir
// Config holds the application configuration
type Config struct {
Token string
ApiUrl string
RepoDir string
PR int
Repo string
Expand Down Expand Up @@ -79,7 +80,7 @@ func New(cfg Config) (*App, error) {
owner := repoSplit[0]
repo := repoSplit[1]

client, err := gh.NewClient(owner, repo, cfg.Token)
client, err := gh.NewClient(owner, repo, cfg.Token, cfg.ApiUrl)
if err != nil {
return nil, err
}
Expand Down
11 changes: 9 additions & 2 deletions internal/github/gh.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,15 @@ type GHClient struct {
infoBuffer io.Writer
}

func NewClient(owner, repo, token string) (Client, error) {
client, err := github.NewClient(github.WithAuthToken(token))
func NewClient(owner, repo, token, apiUrl string) (Client, error) {
opts := []github.ClientOptionsFunc{github.WithAuthToken(token)}
if apiUrl != "" {
// WithEnterpriseURLs appends api/uploads/ to the upload URL, so strip
// any api/v3 suffix to avoid ending up with api/v3/api/uploads/.
uploadUrl := strings.TrimSuffix(strings.TrimSuffix(apiUrl, "/"), "/api/v3")
opts = append(opts, github.WithEnterpriseURLs(apiUrl, uploadUrl))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Broken GHES URL normalization

When a host-only GitHub Enterprise Server URL begins with api., passing it directly to WithEnterpriseURLs prevents /api/v3/ from being appended, causing InitPR and subsequent REST operations to target the host root and fail.

Knowledge Base Used: GitHub API integration

}
Comment on lines +74 to +79

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works for the common cases, but WithEnterpriseURLs guesses the layout from the hostname (e.g. it skips appending /api/v3 for hosts starting with api.), and a doubled trailing slash slips past the single TrimSuffix and yields .../api/v3//api/v3/. If the input defaults to github.api_url (see action.yml comment), the value is already the exact API base URL on every platform, so we can skip the guessing entirely:

apiUrl = strings.TrimSpace(apiUrl)
if apiUrl != "" {
	base, err := url.Parse(strings.TrimRight(apiUrl, "/") + "/")
	if err != nil {
		return nil, fmt.Errorf("invalid api url %q: %w", apiUrl, err)
	}
	if (base.Scheme != "http" && base.Scheme != "https") || base.Host == "" {
		return nil, fmt.Errorf("api url %q must be an absolute http(s) URL", apiUrl)
	}
	client.BaseURL = base
}

(after the plain github.NewClient(github.WithAuthToken(token)) call)

Bonus: this trims stray whitespace and catches schemeless typos like ghe.example.com at startup instead of failing later with an opaque transport error. I'd drop the upload URL handling entirely — nothing in the repo calls upload endpoints.

client, err := github.NewClient(opts...)
if err != nil {
return nil, err
}
Expand Down
75 changes: 73 additions & 2 deletions internal/github/gh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ func TestIsSubstringInComments(t *testing.T) {
}

func TestNewGithubClient(t *testing.T) {
c, err := NewClient("owner", "repo", "token")
c, err := NewClient("owner", "repo", "token", "")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
Expand All @@ -345,6 +345,77 @@ func TestNewGithubClient(t *testing.T) {
if client.userReviewerMap != nil {
t.Error("Expected userReviewerMap to be nil")
}
if got := client.client.BaseURL(); got != "https://api.github.com/" {
t.Errorf("Expected base URL to be https://api.github.com/, got %s", got)
}
Comment on lines +348 to +350

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit (optional): this duplicates the empty uses public GitHub row in the new table below — fine to let the table own default-URL coverage.

}

func TestNewGithubClientApiUrl(t *testing.T) {
tt := []struct {
name string
apiUrl string
expectedBase string
expectedUpload string
expectError bool
}{
{
name: "empty uses public GitHub",
apiUrl: "",
expectedBase: "https://api.github.com/",
expectedUpload: "https://uploads.github.com/",
},
{
name: "enterprise host",
apiUrl: "https://ghe.example.com",
expectedBase: "https://ghe.example.com/api/v3/",
expectedUpload: "https://ghe.example.com/api/uploads/",
},
{
name: "enterprise host with api/v3",
apiUrl: "https://ghe.example.com/api/v3",
expectedBase: "https://ghe.example.com/api/v3/",
expectedUpload: "https://ghe.example.com/api/uploads/",
},
{
name: "enterprise host with api/v3 and trailing slash",
apiUrl: "https://ghe.example.com/api/v3/",
expectedBase: "https://ghe.example.com/api/v3/",
expectedUpload: "https://ghe.example.com/api/uploads/",
},
{
name: "enterprise cloud with data residency",
apiUrl: "https://api.acme.ghe.com",
expectedBase: "https://api.acme.ghe.com/",
expectedUpload: "https://api.acme.ghe.com/",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny note: GitHub's actual upload host for data residency is uploads.<tenant>.ghe.com, so this asserts a value that wouldn't work if we ever add uploads. Since nothing in the repo calls upload endpoints, I'd just drop the expectedUpload column (it goes away naturally if NewClient sets BaseURL directly).

},
{
name: "invalid URL",
apiUrl: "://bad",
expectError: true,
},
}

for _, tc := range tt {
t.Run(tc.name, func(t *testing.T) {
c, err := NewClient("owner", "repo", "token", tc.apiUrl)
if tc.expectError {
if err == nil {
t.Fatal("expected error but got none")
}
return
}
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
gh := c.(*GHClient)
if got := gh.client.BaseURL(); got != tc.expectedBase {
t.Errorf("Expected base URL to be %s, got %s", tc.expectedBase, got)
}
if got := gh.client.UploadURL(); got != tc.expectedUpload {
t.Errorf("Expected upload URL to be %s, got %s", tc.expectedUpload, got)
}
})
}
}

func TestNilPRErr(t *testing.T) {
Expand Down Expand Up @@ -1464,7 +1535,7 @@ func TestContainsValidBypassApproval(t *testing.T) {
}

func TestContainsValidBypassApprovalNoPR(t *testing.T) {
c, err := NewClient("test-owner", "test-repo", "test-token")
c, err := NewClient("test-owner", "test-repo", "test-token", "")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
Expand Down
3 changes: 3 additions & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
// Flags holds the command line flags
type Flags struct {
Token *string
ApiUrl *string
RepoDir *string
PR *int
Repo *string
Expand All @@ -26,6 +27,7 @@ type Flags struct {
var (
flags = &Flags{
Token: flag.String("token", getEnv("INPUT_GITHUB-TOKEN", ""), "GitHub authentication token"),
ApiUrl: flag.String("api-url", getEnv("INPUT_GITHUB-API-URL", ""), "GitHub API base URL (for GitHub Enterprise, e.g. https://ghe.example.com/api/v3)"),
RepoDir: flag.String("dir", getEnv("GITHUB_WORKSPACE", "/"), "Path to local Git repo"),
PR: flag.Int("pr", ignoreError(strconv.Atoi(getEnv("INPUT_PR", ""))), "Pull Request number"),
Repo: flag.String("repo", getEnv("INPUT_REPOSITORY", ""), "GitHub repo name"),
Expand Down Expand Up @@ -127,6 +129,7 @@ func main() {

cfg := app.Config{
Token: *flags.Token,
ApiUrl: *flags.ApiUrl,
RepoDir: *flags.RepoDir,
PR: *flags.PR,
Repo: *flags.Repo,
Expand Down