From 0820df2c4618802a4dc42bea8e76daa2900ea42d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ngh=C4=A9a=20Nguy=E1=BB=85n=20Ng=E1=BB=8Dc?= Date: Sat, 9 May 2026 13:22:58 +0700 Subject: [PATCH 1/2] fix: handle array-typed org/org_id in AuditEntry JSON unmarshaling GitHub's Enterprise audit-log API sometimes returns 'org' as a JSON array of strings and 'org_id' as a JSON array of integers, deviating from the documented scalar types. This caused an unmarshal error: json: cannot unmarshal array into Go struct field ... of type string Fix UnmarshalJSON to capture 'org' and 'org_id' as json.RawMessage, then decode each as scalar-or-array: - string arrays are joined with ', ' - int64 arrays use the first element Add two helpers (unmarshalStringOrStringArray, unmarshalInt64OrInt64Array) and table-driven tests covering scalar, single-element array, and multi-element array payloads. Fixes #3488 --- github/orgs_audit_log.go | 72 +++++++++++++++++++++++++++++- github/orgs_audit_log_test.go | 83 +++++++++++++++++++++++++++++++++++ 2 files changed, 153 insertions(+), 2 deletions(-) diff --git a/github/orgs_audit_log.go b/github/orgs_audit_log.go index 62cffb22e0f..dc28717bb5b 100644 --- a/github/orgs_audit_log.go +++ b/github/orgs_audit_log.go @@ -9,6 +9,7 @@ import ( "context" "encoding/json" "fmt" + "strings" ) // GetAuditLogOptions sets up optional parameters to query audit-log endpoint. @@ -57,12 +58,43 @@ type AuditEntry struct { } // UnmarshalJSON implements the json.Unmarshaler interface. +// +// GitHub's audit-log API occasionally returns "org" as a JSON array of strings +// and "org_id" as a JSON array of integers instead of the documented scalar +// types. This implementation normalises both fields to their scalar forms +// (joining multiple org names with a comma, and using the first org_id) so +// callers always receive a consistent type regardless of the API response shape. func (a *AuditEntry) UnmarshalJSON(data []byte) error { + // rawEntry shadows Org and OrgID so we can inspect their raw JSON tokens + // before deciding how to decode them. type entryAlias AuditEntry - var v entryAlias - if err := json.Unmarshal(data, &v); err != nil { + var raw struct { + entryAlias + Org json.RawMessage `json:"org,omitempty"` + OrgID json.RawMessage `json:"org_id,omitempty"` + } + if err := json.Unmarshal(data, &raw); err != nil { return err } + v := raw.entryAlias + + // Normalise "org": accept both "string" and ["string", ...]. + if len(raw.Org) > 0 && string(raw.Org) != "null" { + org, err := unmarshalStringOrStringArray(raw.Org) + if err != nil { + return fmt.Errorf("AuditEntry.Org: %w", err) + } + v.Org = org + } + + // Normalise "org_id": accept both integer and [integer, ...]. + if len(raw.OrgID) > 0 && string(raw.OrgID) != "null" { + orgID, err := unmarshalInt64OrInt64Array(raw.OrgID) + if err != nil { + return fmt.Errorf("AuditEntry.OrgID: %w", err) + } + v.OrgID = orgID + } rawDefinedFields, err := json.Marshal(v) if err != nil { @@ -90,6 +122,42 @@ func (a *AuditEntry) UnmarshalJSON(data []byte) error { return nil } +// unmarshalStringOrStringArray decodes a JSON value that is either a plain +// string or an array of strings. Arrays are joined with ", ". +func unmarshalStringOrStringArray(raw json.RawMessage) (*string, error) { + // Try scalar string first (the common case). + var s string + if err := json.Unmarshal(raw, &s); err == nil { + return &s, nil + } + // Fall back to array of strings. + var arr []string + if err := json.Unmarshal(raw, &arr); err != nil { + return nil, err + } + joined := strings.Join(arr, ", ") + return &joined, nil +} + +// unmarshalInt64OrInt64Array decodes a JSON value that is either a plain +// integer or an array of integers. Arrays use the first element. +func unmarshalInt64OrInt64Array(raw json.RawMessage) (*int64, error) { + // Try scalar integer first (the common case). + var n int64 + if err := json.Unmarshal(raw, &n); err == nil { + return &n, nil + } + // Fall back to array of integers; use the first element. + var arr []int64 + if err := json.Unmarshal(raw, &arr); err != nil { + return nil, err + } + if len(arr) == 0 { + return nil, nil + } + return &arr[0], nil +} + // MarshalJSON implements the json.Marshaler interface. func (a *AuditEntry) MarshalJSON() ([]byte, error) { type entryAlias AuditEntry diff --git a/github/orgs_audit_log_test.go b/github/orgs_audit_log_test.go index c901ffc6860..d7654b6c1ae 100644 --- a/github/orgs_audit_log_test.go +++ b/github/orgs_audit_log_test.go @@ -386,3 +386,86 @@ func TestAuditEntry_Marshal(t *testing.T) { testJSONMarshalOnly(t, u, want) // can't unmarshal AdditionalFields back into map[string]any, so skip testJSONUnmarshalOnly } + +// TestAuditEntry_UnmarshalJSON_OrgArray verifies that the GitHub Enterprise +// audit-log API's non-standard behaviour of returning "org" as a JSON array +// of strings (instead of a single string) is handled gracefully. +// See: https://github.com/google/go-github/issues/3488 +func TestAuditEntry_UnmarshalJSON_OrgArray(t *testing.T) { + t.Parallel() + tests := []struct { + name string + payload string + wantOrg string + }{ + { + name: "org_as_scalar_string", + payload: `{"action":"test","org":"myorg","org_id":42}`, + wantOrg: "myorg", + }, + { + name: "org_as_single_element_array", + payload: `{"action":"test","org":["myorg"],"org_id":[42]}`, + wantOrg: "myorg", + }, + { + name: "org_as_multi_element_array", + payload: `{"action":"test","org":["org1","org2","org3"],"org_id":[1,2,3]}`, + wantOrg: "org1, org2, org3", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + var entry AuditEntry + if err := entry.UnmarshalJSON([]byte(tc.payload)); err != nil { + t.Fatalf("UnmarshalJSON(%q) returned unexpected error: %v", tc.payload, err) + } + if entry.Org == nil { + t.Fatal("AuditEntry.Org is nil; want non-nil") + } + if *entry.Org != tc.wantOrg { + t.Errorf("AuditEntry.Org = %q; want %q", *entry.Org, tc.wantOrg) + } + }) + } +} + +// TestAuditEntry_UnmarshalJSON_OrgIDArray verifies that the "org_id" field is +// correctly decoded when returned as a JSON array of integers. +func TestAuditEntry_UnmarshalJSON_OrgIDArray(t *testing.T) { + t.Parallel() + tests := []struct { + name string + payload string + wantOrgID int64 + }{ + { + name: "org_id_as_scalar", + payload: `{"action":"test","org_id":42}`, + wantOrgID: 42, + }, + { + name: "org_id_as_array", + payload: `{"action":"test","org_id":[42,43,44]}`, + wantOrgID: 42, // first element + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + var entry AuditEntry + if err := entry.UnmarshalJSON([]byte(tc.payload)); err != nil { + t.Fatalf("UnmarshalJSON(%q) returned unexpected error: %v", tc.payload, err) + } + if entry.OrgID == nil { + t.Fatal("AuditEntry.OrgID is nil; want non-nil") + } + if *entry.OrgID != tc.wantOrgID { + t.Errorf("AuditEntry.OrgID = %d; want %d", *entry.OrgID, tc.wantOrgID) + } + }) + } +} From 38f9ac4fa58bcb063d9172ef71b1b9a4a76e502e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ngh=C4=A9a=20Nguy=E1=BB=85n=20Ng=E1=BB=8Dc?= Date: Sun, 10 May 2026 15:45:30 +0700 Subject: [PATCH 2/2] test: add coverage for error paths and edge cases in UnmarshalJSON MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cover the 5 missing + 6 partial lines flagged by Codecov: - Empty org_id array → nil OrgID (not panic) - Object-typed org → error from unmarshalStringOrStringArray - Object-typed org_id → error from unmarshalInt64OrInt64Array - Explicit JSON null for org and org_id → both fields nil, no error --- github/orgs_audit_log_test.go | 51 +++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/github/orgs_audit_log_test.go b/github/orgs_audit_log_test.go index d7654b6c1ae..fea65f53927 100644 --- a/github/orgs_audit_log_test.go +++ b/github/orgs_audit_log_test.go @@ -469,3 +469,54 @@ func TestAuditEntry_UnmarshalJSON_OrgIDArray(t *testing.T) { }) } } + +// TestAuditEntry_UnmarshalJSON_OrgIDEmptyArray verifies that an empty org_id +// array results in a nil OrgID (not a panic or error). +func TestAuditEntry_UnmarshalJSON_OrgIDEmptyArray(t *testing.T) { + t.Parallel() + var entry AuditEntry + if err := entry.UnmarshalJSON([]byte(`{"action":"test","org_id":[]}`)); err != nil { + t.Fatalf("UnmarshalJSON returned unexpected error: %v", err) + } + if entry.OrgID != nil { + t.Errorf("AuditEntry.OrgID = %d; want nil for empty array", *entry.OrgID) + } +} + +// TestAuditEntry_UnmarshalJSON_InvalidOrgType verifies that a non-string, +// non-array org value (e.g., a JSON object) returns an error. +func TestAuditEntry_UnmarshalJSON_InvalidOrgType(t *testing.T) { + t.Parallel() + var entry AuditEntry + err := entry.UnmarshalJSON([]byte(`{"action":"test","org":{"key":"value"}}`)) + if err == nil { + t.Fatal("UnmarshalJSON should have returned an error for object-typed org, got nil") + } +} + +// TestAuditEntry_UnmarshalJSON_InvalidOrgIDType verifies that a non-integer, +// non-array org_id value (e.g., a JSON object) returns an error. +func TestAuditEntry_UnmarshalJSON_InvalidOrgIDType(t *testing.T) { + t.Parallel() + var entry AuditEntry + err := entry.UnmarshalJSON([]byte(`{"action":"test","org_id":{"key":"value"}}`)) + if err == nil { + t.Fatal("UnmarshalJSON should have returned an error for object-typed org_id, got nil") + } +} + +// TestAuditEntry_UnmarshalJSON_NullOrgFields verifies that explicit JSON null +// values for org and org_id leave the fields as nil without error. +func TestAuditEntry_UnmarshalJSON_NullOrgFields(t *testing.T) { + t.Parallel() + var entry AuditEntry + if err := entry.UnmarshalJSON([]byte(`{"action":"test","org":null,"org_id":null}`)); err != nil { + t.Fatalf("UnmarshalJSON returned unexpected error: %v", err) + } + if entry.Org != nil { + t.Errorf("AuditEntry.Org = %q; want nil for null org", *entry.Org) + } + if entry.OrgID != nil { + t.Errorf("AuditEntry.OrgID = %d; want nil for null org_id", *entry.OrgID) + } +}