[self-care:server-go-logging] Extract token broker policy parsing with diagnostic logging

server/internal/tokenbroker/policy.go

@@ -2,14 +2,18 @@ package tokenbroker
 
 import (
 	"encoding/json"
+	"errors"
 	"io"
 	"os"
 	"regexp"
 	"strings"
 
 	"github.com/githubnext/gh-cao/server/internal/admission"
+	"github.com/githubnext/gh-cao/server/internal/logger"
 )
 
+var brokerLog = logger.New("cao:tokenbroker")
+
 var (
 	stableID = regexp.MustCompile(`^[1-9][0-9]{0,19}$`)
 	revision = regexp.MustCompile(`^[a-f0-9]{40}$`)
@@ -53,14 +57,44 @@ func (s *Service) authorized(i admission.Identity) bool {
 	}
 	// Read on every operation. A changed or removed mounted policy immediately
 	// denies authority instead of retaining a stale authorization snapshot.
-	f, err := os.Open(s.config.PolicyFile)
+	data, err := readPolicyFile(s.config.PolicyFile)
+	if err != nil {
+		brokerLog.Printf("policy denied: reason=unreadable")
+		return false
+	}
+	rules, err := parseWorkflowRules(data)
 	if err != nil {
+		brokerLog.Printf("policy denied: reason=invalid_policy")
+		return false
+	}
+	if !ruleAllows(rules, i) {
+		brokerLog.Printf("policy denied: reason=no_matching_rule rules=%d", len(rules))
 		return false
 	}
+	return true
+}
+
+var errPolicy = errors.New("invalid token broker policy")
+
+// readPolicyFile reads the mounted policy, rejecting files above the 1 MiB bound.
+func readPolicyFile(path string) ([]byte, error) {
+	f, err := os.Open(path)
+	if err != nil {
+		return nil, err
+	}
 	defer func() { _ = f.Close() }()
 	data, err := io.ReadAll(io.LimitReader(f, (1<<20)+1))
-	if err != nil || len(data) > 1<<20 || uniqueJSON(data) != nil {
-		return false
+	if err != nil || len(data) > 1<<20 {
+		return nil, errPolicy
+	}
+	return data, nil
+}
+
+// parseWorkflowRules strictly decodes the copilot-requests workflow rules and
+// fails closed on duplicate members, unknown shapes, duplicates, or unsafe values.
+func parseWorkflowRules(data []byte) ([]workflowRule, error) {
+	if uniqueJSON(data) != nil {
+		return nil, errPolicy
 	}
 	var root, control, broker, requests map[string]json.RawMessage
 	var workflows []json.RawMessage
@@ -69,9 +103,9 @@ func (s *Service) authorized(i admission.Identity) bool {
 		json.Unmarshal(control["token-broker"], &broker) != nil || len(broker) != 1 ||
 		json.Unmarshal(broker["copilot-requests"], &requests) != nil || len(requests) != 1 ||
 		json.Unmarshal(requests["workflows"], &workflows) != nil || len(workflows) > 128 {
-		return false
+		return nil, errPolicy
 	}
-	allowed := false
+	rules := make([]workflowRule, 0, len(workflows))
 	seen := make(map[workflowRule]bool, len(workflows))
 	for _, raw := range workflows {
 		var rule workflowRule
@@ -80,19 +114,27 @@ func (s *Service) authorized(i admission.Identity) bool {
 			fields["repository-id"] == nil || fields["owner-id"] == nil ||
 			fields["workflow-ref"] == nil || fields["workflow-sha"] == nil ||
 			json.Unmarshal(raw, &rule) != nil || seen[rule] {
-			return false
+			return nil, errPolicy
 		}
 		seen[rule] = true
 		if !stableID.MatchString(rule.RepositoryID) || !stableID.MatchString(rule.OwnerID) ||
 			!validWorkflowRef(rule.WorkflowRef) || !revision.MatchString(rule.WorkflowSHA) {
-			return false
+			return nil, errPolicy
 		}
+		rules = append(rules, rule)
+	}
+	return rules, nil
+}
+
+// ruleAllows reports whether an exact rule matches the verified identity.
+func ruleAllows(rules []workflowRule, i admission.Identity) bool {
+	for _, rule := range rules {
 		if rule.RepositoryID == i.RepositoryID && rule.OwnerID == i.OwnerID &&
 			rule.WorkflowRef == i.WorkflowRef && rule.WorkflowSHA == i.WorkflowSHA {
-			allowed = true
+			return true
 		}
 	}
-	return allowed
+	return false
 }
 
 func validWorkflowRef(value string) bool {

server/internal/tokenbroker/policy_rules_test.go

@@ -0,0 +1,103 @@
+package tokenbroker
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/githubnext/gh-cao/server/internal/admission"
+)
+
+const (
+	testRuleRef = "acme/project/.github/workflows/triage.lock.yml@refs/heads/main"
+	testRuleSHA = "0123456789abcdef0123456789abcdef01234567"
+)
+
+func rulesJSON(rule string) []byte {
+	return []byte(`{"control-plane":{"token-broker":{"copilot-requests":{"workflows":[` + rule + `]}}}}`)
+}
+
+func validRule(repo string) string {
+	return `{"repository-id":"` + repo + `","owner-id":"7","workflow-ref":"` + testRuleRef + `","workflow-sha":"` + testRuleSHA + `"}`
+}
+
+func TestParseWorkflowRulesAcceptsValidPolicy(t *testing.T) {
+	rules, err := parseWorkflowRules(rulesJSON(validRule("1") + "," + validRule("2")))
+	if err != nil || len(rules) != 2 || rules[1].RepositoryID != "2" {
+		t.Fatalf("rules=%v err=%v", rules, err)
+	}
+	rules, err = parseWorkflowRules(rulesJSON(""))
+	if err != nil || len(rules) != 0 {
+		t.Fatalf("empty rules=%v err=%v", rules, err)
+	}
+}
+
+func TestParseWorkflowRulesRejectsUnsafePolicy(t *testing.T) {
+	many := strings.TrimSuffix(strings.Repeat(validRule("1")+",", 129), ",")
+	for name, data := range map[string][]byte{
+		"duplicate rule":  rulesJSON(validRule("1") + "," + validRule("1")),
+		"too many":        rulesJSON(many),
+		"wildcard repo":   rulesJSON(validRule("*")),
+		"missing field":   rulesJSON(`{"repository-id":"1","owner-id":"7","workflow-ref":"` + testRuleRef + `"}`),
+		"not an object":   []byte(`[]`),
+		"duplicate field": []byte(`{"control-plane":{},"control-plane":{}}`),
+		"invalid UTF-8":   {0xff},
+	} {
+		if rules, err := parseWorkflowRules(data); err == nil || rules != nil {
+			t.Fatalf("%s: accepted rules=%v", name, rules)
+		}
+	}
+}
+
+func TestRuleAllowsRequiresExactMatch(t *testing.T) {
+	rules, err := parseWorkflowRules(rulesJSON(validRule("1")))
+	if err != nil {
+		t.Fatal(err)
+	}
+	id := admission.Identity{RepositoryID: "1", OwnerID: "7", WorkflowRef: testRuleRef, WorkflowSHA: testRuleSHA}
+	if !ruleAllows(rules, id) {
+		t.Fatal("exact identity denied")
+	}
+	for name, edit := range map[string]func(*admission.Identity){
+		"repository": func(i *admission.Identity) { i.RepositoryID = "2" },
+		"owner":      func(i *admission.Identity) { i.OwnerID = "8" },
+		"ref":        func(i *admission.Identity) { i.WorkflowRef += "x" },
+		"sha":        func(i *admission.Identity) { i.WorkflowSHA = strings.Repeat("a", 40) },
+	} {
+		changed := id
+		edit(&changed)
+		if ruleAllows(rules, changed) {
+			t.Fatalf("%s mismatch authorized", name)
+		}
+	}
+	if ruleAllows(nil, id) {
+		t.Fatal("empty rule set authorized")
+	}
+}
+
+func TestReadPolicyFileBounds(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "policy.json")
+	if _, err := readPolicyFile(path); err == nil {
+		t.Fatal("missing file accepted")
+	}
+	if err := os.WriteFile(path, []byte("{}"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if data, err := readPolicyFile(path); err != nil || string(data) != "{}" {
+		t.Fatalf("data=%q err=%v", data, err)
+	}
+	if err := os.WriteFile(path, make([]byte, (1<<20)+1), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := readPolicyFile(path); err == nil {
+		t.Fatal("oversized policy accepted")
+	}
+	if err := os.WriteFile(path, make([]byte, 1<<20), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := readPolicyFile(path); err != nil {
+		t.Fatalf("policy at the bound rejected: %v", err)
+	}
+}