From 3c337669f8365ecdbf3652ce9f3d77dac7402601 Mon Sep 17 00:00:00 2001 From: rldyourmnd Date: Sat, 12 Sep 2026 22:16:50 +0500 Subject: [PATCH] fix(governance): preserve capability boundaries and valid policy overrides Signed-off-by: rldyourmnd --- core/app/github_readonly.go | 3 + core/compiler/compiler.go | 17 ++++ core/compiler/governance_override_test.go | 37 +++++++++ core/governance/availability_test.go | 78 +++++++++++++++++++ core/governance/governance.go | 17 +++- core/governance/remediation.go | 6 ++ core/providers/github/errors.go | 15 ++++ core/providers/github/governance.go | 6 +- .../github/governance_availability_test.go | 77 ++++++++++++++++++ core/providers/github/types.go | 1 + docs/contracts/policy-v1.md | 19 ++++- 11 files changed, 273 insertions(+), 3 deletions(-) create mode 100644 core/compiler/governance_override_test.go create mode 100644 core/governance/availability_test.go create mode 100644 core/providers/github/governance_availability_test.go diff --git a/core/app/github_readonly.go b/core/app/github_readonly.go index 24f29b0..3fe42d1 100644 --- a/core/app/github_readonly.go +++ b/core/app/github_readonly.go @@ -400,6 +400,9 @@ func githubReadError(command string, installationID string, err error) domain.En case githubprovider.ErrorPermissionContract: class, code = domain.ExitSecurity, "GDS_GITHUB_PERMISSION_CONTRACT_MISMATCH" message = "Effective GitHub App permissions do not exactly match canonical estate intent." + case githubprovider.ErrorCapabilityUnavailable: + class, code = domain.ExitPolicy, "GDS_GITHUB_CAPABILITY_UNAVAILABLE" + message = "The GitHub plan does not provide the requested capability." case githubprovider.ErrorRateLimited, githubprovider.ErrorTransient: class, code = domain.ExitProviderTransient, "GDS_GITHUB_PROVIDER_TRANSIENT" message = "GitHub could not provide current inventory because of a transient provider condition." diff --git a/core/compiler/compiler.go b/core/compiler/compiler.go index 6b66839..7f8588c 100644 --- a/core/compiler/compiler.go +++ b/core/compiler/compiler.go @@ -328,6 +328,23 @@ func (state *mergeState) setLeaf( } setPath(state.effective, parts, value) state.replaceProvenance(parts, value, provenanceFor(source, "set")) + // Observed/ignored contracts do not have a desired value. Moving a higher + // tier away from managed must remove the inherited value and its provenance, + // rather than leave a schema-invalid hybrid behind. An explicit value in + // this same source is still processed and rejected by schema validation. + if len(parts) > 2 && parts[0] == "github" && parts[len(parts)-1] == "management" && + (value == "observed" || value == "ignored") { + parent, exists := lookupPath(state.effective, parts[:len(parts)-1]) + if contract, ok := parent.(map[string]any); exists && ok { + delete(contract, "value") + pointer := jsonPointer(append(append([]string{}, parts[:len(parts)-1]...), "value")) + for existing := range state.provenance { + if existing == pointer || strings.HasPrefix(existing, pointer+"/") { + delete(state.provenance, existing) + } + } + } + } } func (state *mergeState) replaceProvenance(parts []string, value any, provenance Provenance) { diff --git a/core/compiler/governance_override_test.go b/core/compiler/governance_override_test.go new file mode 100644 index 0000000..77b8a6d --- /dev/null +++ b/core/compiler/governance_override_test.go @@ -0,0 +1,37 @@ +package compiler + +import ( + "strings" + "testing" +) + +func TestGovernanceManagementOverrideClearsInheritedDesiredValue(t *testing.T) { + for _, mode := range []string{"observed", "ignored"} { + t.Run(mode, func(t *testing.T) { + base := testSource("base", "base", 100, map[string]any{"github": map[string]any{"merge": map[string]any{ + "squash_merge_commit_title": map[string]any{"management": "managed", "value": "PR_TITLE"}, + }}}) + leaf := testSource("leaf", "repository", 100, map[string]any{"github": map[string]any{"merge": map[string]any{ + "squash_merge_commit_title": map[string]any{"management": mode}, + }}}) + result := New(testSchemas(t)).Compile(testAnchor("base", "leaf"), map[string]PolicySource{"base": base, "leaf": leaf}, DevelopmentBundleVersion) + if len(result.Findings) != 0 { + t.Fatalf("valid management override refused: %#v", result.Findings) + } + value, ok := lookupPath(result.Document.Effective, strings.Split("github.merge.squash_merge_commit_title", ".")) + if !ok { + t.Fatal("overridden contract is absent") + } + contract := value.(map[string]any) + if _, exists := contract["value"]; exists { + t.Fatalf("%s inherited a desired value: %#v", mode, contract) + } + if _, exists := result.Document.Provenance["/effective/github/merge/squash_merge_commit_title/value"]; exists { + t.Fatal("removed desired value retained provenance") + } + if result.Document.Provenance["/effective/github/merge/squash_merge_commit_title/management"].Source != "leaf" { + t.Fatal("management does not bind the overriding source") + } + }) + } +} diff --git a/core/governance/availability_test.go b/core/governance/availability_test.go new file mode 100644 index 0000000..38906b5 --- /dev/null +++ b/core/governance/availability_test.go @@ -0,0 +1,78 @@ +package governance + +import ( + "context" + "github.com/NDDev-OpenNetwork/github-device-sync/core/compiler" + "testing" +) + +func availabilityPolicy(mode string) compiler.CompiledPolicyDocument { + return compiler.CompiledPolicyDocument{ + CompiledPolicy: compiler.CompiledPolicyMetadata{Digest: "sha256:policy"}, + Effective: map[string]any{"github": map[string]any{ + "merge": map[string]any{"allow_squash_merge": map[string]any{"management": "managed", "value": true}}, + "rulesets": map[string]any{"management": mode}, + }}, + } +} + +func TestAvailableSettingsCanBeAppliedWithExplicitUnavailableRulesets(t *testing.T) { + snapshot := governanceTestSnapshot() + snapshot.Repository.Merge.AllowSquashMerge = false + snapshot.Unavailable = map[string]string{"github.rulesets": "plan-restricted"} + comparison := Compare(availabilityPolicy("observed"), snapshot) + if comparison.Counts["unavailable"] != 1 || comparison.Counts["drift"] != 1 { + t.Fatalf("comparison=%#v", comparison) + } + for _, f := range comparison.Fields { + if f.Path == "github.rulesets" && (f.Observed != nil || f.Reason != "plan-restricted") { + t.Fatalf("unknown rulesets presented as observed: %#v", f) + } + } + remediation, err := BuildRemediation(governanceTestScope(), snapshot, comparison) + if err != nil { + t.Fatal(err) + } + if len(remediation.Steps) != 1 || remediation.Steps[0].Action != RepositorySettingsAction { + t.Fatalf("steps=%#v", remediation.Steps) + } + fixture := &governanceFixture{snapshot: snapshot} + handler := &Handler{Reader: fixture, Writer: fixture, Scope: fixture.Scope(), Action: RepositorySettingsAction} + if _, err := handler.Apply(context.Background(), remediation.Steps[0]); err != nil { + t.Fatal(err) + } + if fixture.writes != 1 || !fixture.snapshot.Repository.Merge.AllowSquashMerge { + t.Fatalf("available field not updated: %#v", fixture) + } + final := Compare(availabilityPolicy("observed"), fixture.snapshot) + if final.Status != "partially-observed" || final.Counts["compliant"] != 1 || final.Counts["unavailable"] != 1 { + t.Fatalf("partial coverage hidden: %#v", final) + } +} + +func TestManagedUnavailableFieldCannotProduceMutationPlan(t *testing.T) { + snapshot := governanceTestSnapshot() + snapshot.Unavailable = map[string]string{"github.rulesets": "plan-restricted"} + comparison := Compare(availabilityPolicy("managed"), snapshot) + if _, err := BuildRemediation(governanceTestScope(), snapshot, comparison); err == nil { + t.Fatal("managed unavailable field was accepted") + } +} + +func TestUnavailableEvidenceIsImmutableAndDoesNotChangeHistoricalDigests(t *testing.T) { + snapshot := governanceTestSnapshot() + before := mustGovernanceDigest(t, snapshot) + snapshot.Unavailable = map[string]string{} + if mustGovernanceDigest(t, snapshot) != before { + t.Fatal("empty availability metadata changed historical digest") + } + snapshot.Unavailable["github.rulesets"] = "plan-restricted" + if mustGovernanceDigest(t, snapshot) == before { + t.Fatal("unavailable rulesets indistinguishable from empty rulesets") + } + stable := Stabilize(snapshot) + snapshot.Unavailable["github.rulesets"] = "changed" + if stable.Unavailable["github.rulesets"] != "plan-restricted" { + t.Fatal("stable evidence aliases mutable input") + } +} diff --git a/core/governance/governance.go b/core/governance/governance.go index 411f2ef..068b4a1 100644 --- a/core/governance/governance.go +++ b/core/governance/governance.go @@ -23,6 +23,7 @@ type StableSnapshot struct { Workflow githubprovider.WorkflowPermissions `json:"workflow"` ImmutableReleases githubprovider.ImmutableReleases `json:"immutable_releases"` Rulesets []githubprovider.RulesetSummary `json:"rulesets"` + Unavailable map[string]string `json:"unavailable,omitempty"` } type StableRepository struct { @@ -40,6 +41,13 @@ type StableRepository struct { } func Stabilize(snapshot githubprovider.GovernanceSnapshot) StableSnapshot { + var unavailable map[string]string + if len(snapshot.Unavailable) != 0 { + unavailable = make(map[string]string, len(snapshot.Unavailable)) + for path, reason := range snapshot.Unavailable { + unavailable[path] = reason + } + } rulesets := append([]githubprovider.RulesetSummary(nil), snapshot.Rulesets...) sort.Slice(rulesets, func(left, right int) bool { return rulesets[left].ID < rulesets[right].ID }) features := make(map[string]string, len(snapshot.Repository.Security.Features)) @@ -67,7 +75,7 @@ func Stabilize(snapshot githubprovider.GovernanceSnapshot) StableSnapshot { }, Actions: snapshot.Actions, SelectedActions: selected, Workflow: snapshot.Workflow, ImmutableReleases: snapshot.ImmutableReleases, - Rulesets: rulesets, + Rulesets: rulesets, Unavailable: unavailable, } } @@ -85,6 +93,7 @@ type FieldResult struct { Status string `json:"status"` Desired any `json:"desired,omitempty"` Observed any `json:"observed,omitempty"` + Reason string `json:"reason,omitempty"` } type Result struct { @@ -152,6 +161,10 @@ func Compare( default: entry.Status = "invalid-policy" } + if reason := snapshot.Unavailable[field.path]; reason != "" && + (management == "managed" || management == "observed") { + entry.Status, entry.Reason, entry.Observed = "unavailable", reason, nil + } result.Counts[entry.Status]++ result.Fields = append(result.Fields, entry) } @@ -163,6 +176,8 @@ func Compare( result.Status = "invalid-policy" case result.Counts["drift"] != 0: result.Status = "drift" + case result.Counts["unavailable"] != 0: + result.Status = "partially-observed" case result.Counts["compliant"] != 0: result.Status = "compliant" case result.Counts["observed"] != 0 || result.Counts["ignored"] != 0: diff --git a/core/governance/remediation.go b/core/governance/remediation.go index 3732ade..656845b 100644 --- a/core/governance/remediation.go +++ b/core/governance/remediation.go @@ -46,6 +46,12 @@ func BuildRemediation( if comparison.Status == "invalid-policy" || comparison.PolicyDigest == "" { return Remediation{}, errors.New("GitHub governance remediation requires one valid compiled policy") } + for _, field := range comparison.Fields { + if field.Management == "managed" && + (field.Status == "unavailable" || snapshot.Unavailable[field.Path] != "") { + return Remediation{}, fmt.Errorf("managed governance field %s is unavailable; no mutation can be planned", field.Path) + } + } initialDigest, err := EvidenceDigest(snapshot) if err != nil { return Remediation{}, err diff --git a/core/providers/github/errors.go b/core/providers/github/errors.go index 88bb0ba..612ad27 100644 --- a/core/providers/github/errors.go +++ b/core/providers/github/errors.go @@ -2,6 +2,7 @@ package github import ( "bytes" + "encoding/json" "errors" "fmt" "strings" @@ -13,6 +14,7 @@ type ErrorKind string const ( ErrorAuthentication ErrorKind = "authentication" ErrorAuthorization ErrorKind = "authorization" + ErrorCapabilityUnavailable ErrorKind = "capability-unavailable" ErrorPermissionContract ErrorKind = "permission-contract" ErrorNotFoundOrInaccessible ErrorKind = "not-found-or-inaccessible" ErrorRateLimited ErrorKind = "rate-limited" @@ -77,6 +79,8 @@ func classifyStatus(status int, body []byte, meta ResponseMeta) ErrorKind { return ErrorAuthentication case status == 403 && (meta.RetryAfter > 0 || (meta.Rate.Known && meta.Rate.Remaining == 0) || isSecondaryRateLimitBody(body)): return ErrorRateLimited + case status == 403 && isPlanRestrictionBody(body): + return ErrorCapabilityUnavailable case status == 403: return ErrorAuthorization case status == 404: @@ -94,6 +98,17 @@ func classifyStatus(status int, body []byte, meta ResponseMeta) ErrorKind { } } +// Recognize the provider's explicit product restriction, never an arbitrary +// 403, which may instead mean revoked access or a secondary rate limit. The +// response body stays private; only a bounded classification leaves the client. +func isPlanRestrictionBody(body []byte) bool { + var response struct { + Message string `json:"message"` + } + return json.Unmarshal(body, &response) == nil && response.Message == + "Upgrade to GitHub Pro or make this repository public to enable this feature." +} + // isSecondaryRateLimitBody reports whether the response body indicates a // GitHub "secondary rate limit" (an abuse-detection throttle), which is // returned as a 403 without the standard remaining-quota metadata. diff --git a/core/providers/github/governance.go b/core/providers/github/governance.go index 9556270..617086e 100644 --- a/core/providers/github/governance.go +++ b/core/providers/github/governance.go @@ -70,7 +70,11 @@ func (client *Client) GetRepositoryGovernance( appendMeta(repositoryMeta) snapshot.Rulesets, repositoryMeta, err = client.getRulesets(ctx, base) if err != nil { - return GovernanceSnapshot{}, err + var apiError *APIError + if !errors.As(err, &apiError) || apiError.Kind != ErrorCapabilityUnavailable { + return GovernanceSnapshot{}, err + } + snapshot.Unavailable = map[string]string{"github.rulesets": "plan-restricted"} } appendMeta(repositoryMeta) return snapshot, nil diff --git a/core/providers/github/governance_availability_test.go b/core/providers/github/governance_availability_test.go new file mode 100644 index 0000000..8cb675c --- /dev/null +++ b/core/providers/github/governance_availability_test.go @@ -0,0 +1,77 @@ +package github + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" +) + +func TestGovernancePlanRestrictionPreservesAvailableRepositorySettings(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/repos/example/repository": + _, _ = w.Write([]byte(repositoryJSON(1, "example", "repository", false))) + case "/repos/example/repository/actions/permissions": + _, _ = w.Write([]byte(`{"enabled":true,"allowed_actions":"all"}`)) + case "/repos/example/repository/actions/permissions/workflow": + _, _ = w.Write([]byte(`{"default_workflow_permissions":"read"}`)) + case "/repos/example/repository/immutable-releases": + http.NotFound(w, r) + case "/repos/example/repository/rulesets": + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"message":"Upgrade to GitHub Pro or make this repository public to enable this feature."}`)) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + now := time.Now() + client := testClient(t, server, fixedToken("token", now.Add(time.Hour)), nil) + snapshot, err := client.GetRepositoryGovernance(context.Background(), "example", "repository") + if err != nil { + t.Fatalf("unavailable rulesets erased available settings: %v", err) + } + if snapshot.Repository.ID != 1 || !snapshot.Actions.Enabled { + t.Fatalf("available observation lost: %#v", snapshot) + } + raw, err := json.Marshal(snapshot) + if err != nil { + t.Fatal(err) + } + var output map[string]any + if err := json.Unmarshal(raw, &output); err != nil { + t.Fatal(err) + } + unavailable, ok := output["unavailable"].(map[string]any) + if !ok || unavailable["github.rulesets"] != "plan-restricted" { + t.Fatalf("plan restriction is not explicit: %s", raw) + } +} + +func TestPlanRestrictionNeverMasksAuthorizationOrRateLimits(t *testing.T) { + plan := []byte(`{"message":"Upgrade to GitHub Pro or make this repository public to enable this feature."}`) + for _, tc := range []struct { + name string + status int + body []byte + meta ResponseMeta + want ErrorKind + }{ + {"plan", 403, plan, ResponseMeta{}, ErrorCapabilityUnavailable}, + {"authentication", 401, plan, ResponseMeta{}, ErrorAuthentication}, + {"access revoked", 403, []byte(`{"message":"Resource not accessible by integration"}`), ResponseMeta{}, ErrorAuthorization}, + {"malformed", 403, []byte(`not JSON`), ResponseMeta{}, ErrorAuthorization}, + {"secondary throttle", 403, []byte(`{"message":"You have exceeded a secondary rate limit."}`), ResponseMeta{}, ErrorRateLimited}, + {"retry header", 403, plan, ResponseMeta{RetryAfter: time.Second}, ErrorRateLimited}, + {"exhausted quota", 403, plan, ResponseMeta{Rate: Rate{Known: true, Remaining: 0}}, ErrorRateLimited}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := classifyStatus(tc.status, tc.body, tc.meta); got != tc.want { + t.Fatalf("kind=%s want %s", got, tc.want) + } + }) + } +} diff --git a/core/providers/github/types.go b/core/providers/github/types.go index 6d5301c..729405d 100644 --- a/core/providers/github/types.go +++ b/core/providers/github/types.go @@ -130,6 +130,7 @@ type GovernanceSnapshot struct { Workflow WorkflowPermissions `json:"workflow"` ImmutableReleases ImmutableReleases `json:"immutable_releases"` Rulesets []RulesetSummary `json:"rulesets"` + Unavailable map[string]string `json:"unavailable,omitempty"` Permissions PermissionEvidence `json:"permissions"` ObservedAt time.Time `json:"observed_at"` Rate Rate `json:"rate"` diff --git a/docs/contracts/policy-v1.md b/docs/contracts/policy-v1.md index 1e03f30..9e482f6 100644 --- a/docs/contracts/policy-v1.md +++ b/docs/contracts/policy-v1.md @@ -59,13 +59,30 @@ appearance. Repository GitHub governance is declared under `apply.github`. Each setting is explicitly `managed`, `observed`, or `ignored`; only `managed` carries a -desired value. Selected Actions are one atomic contract containing +desired value. A later `observed` or `ignored` management override removes the +inherited desired value and its leaf provenance. A source that explicitly +supplies a value with either mode is still invalid. Selected Actions are one atomic contract containing `github_owned_allowed`, `verified_allowed`, and the complete normalized pattern allowlist. `github.releases.immutable` controls repository-level immutable-release enablement; owner enforcement is observed provider state and cannot be weakened by repository policy. The read-only governance comparator never turns observed or ignored evidence into a remediation target. +Provider capabilities can be unavailable independently of repository access. +The explicit GitHub plan restriction on the rulesets endpoint produces +`unavailable["github.rulesets"] = "plan-restricted"` in the snapshot. The +comparator reports that field as `unavailable`, not as an empty or compliant +ruleset set, while retaining available repository settings. With no other drift, +the overall result is `partially-observed`. Available managed fields can still +be planned and exactly verified; an unavailable managed field refuses the +mutation plan. Ordinary authorization errors, malformed responses and rate +limits continue to fail closed. + +Availability is part of the stable optimistic-concurrency evidence digest. +Absent availability metadata is omitted, preserving historical digests for +fully observed snapshots. A product/permission change between plan and apply +therefore cannot reuse an observation with a different capability boundary. + Policy-source references are not part of schema v1, so cycles are not representable. If references are added later, cycle detection becomes a schema and compiler release gate.