Enterprise API responses are sometimes returning arrays of strings instead of a string

[antixcode6]

Context

go version go1.24.0 darwin/arm64
Github Enterprise REST API
go-github versions v66 and v69 tested with

The issue

While pulling logs from the GH enterprise REST API via client.Enterprise.GetAuditLog(ctx, enterprise-name, &opts) it seems that there are some issues with actual API response vs the Github provided response schema for /enterprises/{enterprise}/audit-log

The docs state org should be a string

https://docs.github.com/en/enterprise-cloud@latest/rest/enterprise-admin/audit-log?apiVersion=2022-11-28#get-the-audit-log-for-an-enterprise

      "org": {
        "type": "string"
      }

However, while pulling audit logs from an enterprise account the org field may sometimes be an array of strings.

{
  "@timestamp" : 1739983846824,
  "_document_id" : "<redacted>",
  "action" : "oauth_authorization.create",
  "actor" : "<redacted>",
  "actor_id" : 123456789,
  "actor_ip" : "<redacted>",
  "actor_is_bot" : false,
  "actor_location" : {
    "country_code" : "GB"
  },
  "business" : "<redacted>",
  "business_id" : 123456,
  "created_at" : 123456789,
  "external_identity_username" : "<redacted>",
  "operation_type" : "create",
  "org" : [ "<redacted>", "<redacted>", "<redacted>" ],
  "org_id" : [ 123456789, 123456789, 123456789 ],
  "request_access_security_header" : null,
  "token_scopes" : "gist,repo,workflow",
  "user" : "<redacted>",
  "user_agent" : "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/133.0.0.0 Safari/537.36 Edg/133.0.0.0",
  "user_id" : 123456789
}

This is causing the following error when trying to unmarshal the response into a struct json: cannot unmarshal array into Go struct field entryAlias.org of type string

This error is present in v66 and v69, I've tested and was able to replicate the error with both versions.

Is this something this package may be able to flex? I wonder if the array can be turned into a string with strings.Join() when an array is detected during the type switch in this function https://github.com/google/go-github/blob/master/github/github.go#L1024-L1045

Specifically it is failing here

	default:
		decErr := json.NewDecoder(resp.Body).Decode(v)
		if decErr == io.EOF {
			decErr = nil // ignore EOF errors caused by empty response body
		}
		if decErr != nil {
			err = decErr
		}
	}

as it cannot decode the body properly.

[gmlewis]

Yes, I think we could, upon failure, see if the array could be turned into a comma-separated string and returned that way without breaking the API. The only danger might be if an enterprise name already had a comma in it, like "Glenn's Company, Inc." then the comma-separated list would not be good.

Another alternative would be to create a breaking-API change and change that field from type string to type json.RawMessage, then the user of that field could test it for being a string or being a []string and handle it accordingly.

Thoughts?

Would you like to write a PR?

[antixcode6]

@gmlewis I started looking into using json.RawMessage one thing I see as a potential issue are these accessor functions needing to be modified func (a *AuditEntry) GetOrg() and func (a *AuditEntry) GetOrgID() as I did not realize until I started working on this that, to no surprise, orgID is also a slice of int64.

There are some tests that would need modification as well -- I am not against this but I just wanted to make sure this all makes sense before continuing/creating a PR

some sample changes to the accessor functions

// GetOrg returns the Org field if it's non-nil, zero value otherwise.
func (a *AuditEntry) GetOrg() string {
	var org *string

	if a == nil || a.Org == nil {
		return ""
	}

	json.Unmarshal([]byte(a.Org), org) // unmarsal the json.rawmessage to string

	return *org
}

// GetOrgID returns the OrgID field if it's non-nil, zero value otherwise.
func (a *AuditEntry) GetOrgID() int64 {
	var org_id *int64

	if a == nil || a.OrgID == nil {
		return 0
	}

	json.Unmarshal([]byte(a.OrgID), org_id) // unmarsal the json.rawmessage to int64

	return *org_id
}

[gmlewis]

Well, the first thing that strikes me is that I would not write the accessors that way.
If we write accessors, they would be more like this so that you can absolutely tell if they were successful:

// GetOrg returns the Org field if it's valid.
func (a *AuditEntry) GetOrg() (org string, ok bool) {
	if a == nil || a.Org == nil {
		return "", false
	}
	if err := json.Unmarshal([]byte(a.Org), &org); err != nil {
		return "", false
	}

	return org, true
}

// GetOrgID returns the OrgID field if it's valid.
func (a *AuditEntry) GetOrgID() (orgID int64, ok bool) {
	if a == nil || a.OrgID == nil {
		return 0, false
	}
	if err := json.Unmarshal([]byte(a.OrgID), &orgID); err != nil {
		return 0, false
	}

	return orgID, true
}

However, to not interfere with the existing auto-generated accessor mechanisms already established, we might need to consider other alternatives. I think we have a skipStructMethods mechanism to avoid writing accessors for particular fields, and maybe that is sufficient, but I would have to investigate. Maybe you could do that.

[antixcode6]

Hello @gmlewis I modified skipStructMethods to the following

	// skipStructMethods lists "struct.method" combos to skip.
	skipStructMethods = map[string]bool{
		"RepositoryContent.GetContent":    true,
		"Client.GetBaseURL":               true,
		"Client.GetUploadURL":             true,
		"ErrorResponse.GetResponse":       true,
		"RateLimitError.GetResponse":      true,
		"AbuseRateLimitError.GetResponse": true,
		"AuditEntry.GetOrgID":             true,
		"AuditEntry.GetOrg":               true,
	}

and ran go generate ./... which looks promising.

I had to modify the non-generated tests in orgs_audit_log_test.go and enterprise_audit_log_test.go to work with the new json.RawMessage fields but everything is passing so I think this is ok -- I would love to hear your thoughts though.

	orgID, _ := json.Marshal(Ptr(int64(1)))

	want := []*AuditEntry{
		{
			Timestamp:  &Timestamp{timestamp},
			DocumentID: Ptr("beeZYapIUe-wKg5-beadb33"),
			Action:     Ptr("workflows.completed_workflow_run"),
			Actor:      Ptr("testactor"),
			ActorLocation: &ActorLocation{
				CountryCode: Ptr("US"),
			},
			CreatedAt:   &Timestamp{timestamp},
			HashedToken: Ptr("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA="),
			Org:         json.RawMessage(bytes.Join([][]byte{[]byte(`"o"`), []byte("")}, []byte(""))),
			OrgID:       orgID,

[gmlewis]

It is really hard to do code reviews without context diffs.
Although it sounds like what you are describing is a valid way of moving forward, I can't fully tell until I see a PR.
Therefore, please put together a PR and we can discuss the details there.
Thank you, @antixcode6!

[antixcode6]

Thank you for the guidance @gmlewis!

I've opened a PR #3502