From eb432a485e9c5e2b3b765bf0ef9e8f2bc31caa76 Mon Sep 17 00:00:00 2001 From: MK Date: Wed, 13 May 2026 14:07:34 -0400 Subject: [PATCH] feat(analyzer): policy-configurable security scoring (closes #49) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thread SecurityPolicy through AnalyzeSkillDescriptor / AnalyzeSkillEntry so operators can adjust scoring for custom skills without editing any file under forge-skills/analyzer/. - types.go: extend SecurityPolicy with AcknowledgedBins and AcknowledgedEnv; document TrustedDomains plus the two new fields as scoring overrides distinct from policy-check fields. - scoring.go: wire policy.TrustedDomains into scoreEgress (was always passed nil — the existing wiring gap). Add policy-aware variants for scoreBinaries and scoreEnv so AcknowledgedBins / AcknowledgedEnv lower the points and mark the RiskFactor description with "(acknowledged by policy)" or "(via policy)" so the override is visible in audit output. Builtin classifications still win to preserve audit consistency across projects. - policy_loader.go: LoadPolicyFromFile parses a YAML SecurityPolicy. - cmd/skills.go: --policy flag on `forge skills audit` replaces DefaultPolicy() with the YAML-loaded one for both scoring and policy checks. - Tests: full coverage of overrides (TrustedDomains, AcknowledgedBins, AcknowledgedEnv), confirmation that builtin trust isn't re-attributed to policy, that AcknowledgedBins doesn't promote standard binaries, and a YAML loader round-trip including the end-to-end scoring effect. --- forge-cli/cmd/skills.go | 9 ++ forge-skills/analyzer/policy.go | 2 +- forge-skills/analyzer/policy_loader.go | 23 ++++ forge-skills/analyzer/policy_loader_test.go | 95 ++++++++++++++ forge-skills/analyzer/report.go | 2 +- forge-skills/analyzer/scoring.go | 87 +++++++++---- forge-skills/analyzer/scoring_test.go | 131 ++++++++++++++++++-- forge-skills/analyzer/types.go | 13 +- 8 files changed, 322 insertions(+), 40 deletions(-) create mode 100644 forge-skills/analyzer/policy_loader.go create mode 100644 forge-skills/analyzer/policy_loader_test.go diff --git a/forge-cli/cmd/skills.go b/forge-cli/cmd/skills.go index f594773..38a0efa 100644 --- a/forge-cli/cmd/skills.go +++ b/forge-cli/cmd/skills.go @@ -77,6 +77,7 @@ var skillsTrustReportCmd = &cobra.Command{ var auditFormat string var auditEmbedded bool var auditDir string +var auditPolicyPath string var signKeyPath string func init() { @@ -91,6 +92,7 @@ func init() { skillsAuditCmd.Flags().StringVar(&auditFormat, "format", "text", "Output format: text or json") skillsAuditCmd.Flags().BoolVar(&auditEmbedded, "embedded", false, "Audit embedded skills from the binary") skillsAuditCmd.Flags().StringVar(&auditDir, "dir", "", "Audit skills from a directory of SKILL.md subdirectories") + skillsAuditCmd.Flags().StringVar(&auditPolicyPath, "policy", "", "Path to a YAML security policy file (overrides DefaultPolicy for both scoring and policy checks)") skillsSignCmd.Flags().StringVar(&signKeyPath, "key", "", "Path to Ed25519 private key") _ = skillsSignCmd.MarkFlagRequired("key") } @@ -375,6 +377,13 @@ func loadSecretPlaceholders(path string) map[string]bool { func runSkillsAudit(cmd *cobra.Command, args []string) error { policy := analyzer.DefaultPolicy() + if auditPolicyPath != "" { + loaded, err := analyzer.LoadPolicyFromFile(auditPolicyPath) + if err != nil { + return err + } + policy = loaded + } var report *analyzer.AuditReport switch { diff --git a/forge-skills/analyzer/policy.go b/forge-skills/analyzer/policy.go index 333f588..bb0b8db 100644 --- a/forge-skills/analyzer/policy.go +++ b/forge-skills/analyzer/policy.go @@ -98,7 +98,7 @@ func CheckPolicy(sd *contract.SkillDescriptor, hasScript bool, policy SecurityPo // Rule 6: MaxRiskScore if policy.MaxRiskScore > 0 { - assessment := AnalyzeSkillDescriptor(sd, hasScript) + assessment := AnalyzeSkillDescriptor(sd, hasScript, policy) if assessment.Score.Value > policy.MaxRiskScore { violations = append(violations, PolicyViolation{ Rule: "max_risk_score", diff --git a/forge-skills/analyzer/policy_loader.go b/forge-skills/analyzer/policy_loader.go new file mode 100644 index 0000000..28523c6 --- /dev/null +++ b/forge-skills/analyzer/policy_loader.go @@ -0,0 +1,23 @@ +package analyzer + +import ( + "fmt" + "os" + + "gopkg.in/yaml.v3" +) + +// LoadPolicyFromFile reads a YAML SecurityPolicy from path. Unspecified fields +// take their zero value, which means no override is applied — a minimal policy +// file can omit any rule it doesn't intend to change. +func LoadPolicyFromFile(path string) (SecurityPolicy, error) { + var p SecurityPolicy + data, err := os.ReadFile(path) + if err != nil { + return p, fmt.Errorf("reading policy file %q: %w", path, err) + } + if err := yaml.Unmarshal(data, &p); err != nil { + return p, fmt.Errorf("parsing policy file %q: %w", path, err) + } + return p, nil +} diff --git a/forge-skills/analyzer/policy_loader_test.go b/forge-skills/analyzer/policy_loader_test.go new file mode 100644 index 0000000..9b68536 --- /dev/null +++ b/forge-skills/analyzer/policy_loader_test.go @@ -0,0 +1,95 @@ +package analyzer + +import ( + "os" + "path/filepath" + "slices" + "testing" +) + +func TestLoadPolicyFromFile(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "policy.yaml") + yaml := ` +script_policy: allow +max_risk_score: 90 +trusted_domains: + - internal.example.com + - corp.example.com +acknowledged_bins: + - python + - python3 +acknowledged_env: + - DB_PASSWORD +` + if err := os.WriteFile(path, []byte(yaml), 0o644); err != nil { + t.Fatalf("writing policy file: %v", err) + } + + p, err := LoadPolicyFromFile(path) + if err != nil { + t.Fatalf("LoadPolicyFromFile: %v", err) + } + + if p.ScriptPolicy != "allow" { + t.Errorf("ScriptPolicy = %q, want %q", p.ScriptPolicy, "allow") + } + if p.MaxRiskScore != 90 { + t.Errorf("MaxRiskScore = %d, want 90", p.MaxRiskScore) + } + if !slices.Equal(p.TrustedDomains, []string{"internal.example.com", "corp.example.com"}) { + t.Errorf("TrustedDomains = %v", p.TrustedDomains) + } + if !slices.Equal(p.AcknowledgedBins, []string{"python", "python3"}) { + t.Errorf("AcknowledgedBins = %v", p.AcknowledgedBins) + } + if !slices.Equal(p.AcknowledgedEnv, []string{"DB_PASSWORD"}) { + t.Errorf("AcknowledgedEnv = %v", p.AcknowledgedEnv) + } +} + +// TestLoadPolicyFromFile_PolicyAffectsScoring is an end-to-end check that a +// policy loaded from YAML actually changes the assessment — the regression +// the existing wiring gap created and #49 closes. +func TestLoadPolicyFromFile_PolicyAffectsScoring(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "policy.yaml") + yaml := "trusted_domains: [internal.example.com]\nacknowledged_bins: [python]\n" + if err := os.WriteFile(path, []byte(yaml), 0o644); err != nil { + t.Fatalf("writing policy file: %v", err) + } + + policy, err := LoadPolicyFromFile(path) + if err != nil { + t.Fatalf("LoadPolicyFromFile: %v", err) + } + + factors := scoreEgress([]string{"internal.example.com"}, policy) + if len(factors) != 1 || factors[0].Points != 2 { + t.Errorf("scoreEgress with loaded TrustedDomains: factors=%v", factors) + } + + factors = scoreBinaries([]string{"python"}, policy) + if len(factors) != 1 || factors[0].Points != 3 { + t.Errorf("scoreBinaries with loaded AcknowledgedBins: factors=%v", factors) + } +} + +func TestLoadPolicyFromFile_MissingFile(t *testing.T) { + _, err := LoadPolicyFromFile(filepath.Join(t.TempDir(), "missing.yaml")) + if err == nil { + t.Fatal("expected error for missing file") + } +} + +func TestLoadPolicyFromFile_InvalidYAML(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "bad.yaml") + if err := os.WriteFile(path, []byte("this: is: not: valid: yaml: ["), 0o644); err != nil { + t.Fatalf("writing bad file: %v", err) + } + _, err := LoadPolicyFromFile(path) + if err == nil { + t.Fatal("expected parse error for invalid YAML") + } +} diff --git a/forge-skills/analyzer/report.go b/forge-skills/analyzer/report.go index 450ae72..db768c3 100644 --- a/forge-skills/analyzer/report.go +++ b/forge-skills/analyzer/report.go @@ -73,7 +73,7 @@ func GenerateReportFromEntries(entries []contract.SkillEntry, hasScript func(str entry := &entries[i] hs := hasScript != nil && hasScript(entry.Name) - assessment := AnalyzeSkillEntry(entry, hs) + assessment := AnalyzeSkillEntry(entry, hs, policy) // Run policy checks violations := CheckPolicyFromEntry(entry, hs, policy) diff --git a/forge-skills/analyzer/scoring.go b/forge-skills/analyzer/scoring.go index 8ea0462..bf5f3b0 100644 --- a/forge-skills/analyzer/scoring.go +++ b/forge-skills/analyzer/scoring.go @@ -46,13 +46,15 @@ var sensitiveEnvPatterns = []string{ "CREDENTIALS", } -// AnalyzeSkillDescriptor scores a SkillDescriptor for security risk. -func AnalyzeSkillDescriptor(sd *contract.SkillDescriptor, hasScript bool) SkillRiskAssessment { +// AnalyzeSkillDescriptor scores a SkillDescriptor for security risk under the +// given policy. A zero-value SecurityPolicy{} preserves the historical default +// scoring (no overrides applied). +func AnalyzeSkillDescriptor(sd *contract.SkillDescriptor, hasScript bool, policy SecurityPolicy) SkillRiskAssessment { var factors []RiskFactor - factors = append(factors, scoreEgress(sd.EgressDomains, nil)...) - factors = append(factors, scoreBinaries(sd.RequiredBins)...) - factors = append(factors, scoreEnv(sd.RequiredEnv, sd.OneOfEnv, sd.OptionalEnv)...) + factors = append(factors, scoreEgress(sd.EgressDomains, policy)...) + factors = append(factors, scoreBinaries(sd.RequiredBins, policy)...) + factors = append(factors, scoreEnv(sd.RequiredEnv, sd.OneOfEnv, sd.OptionalEnv, policy)...) if hasScript { factors = append(factors, scoreScript()...) } @@ -66,8 +68,10 @@ func AnalyzeSkillDescriptor(sd *contract.SkillDescriptor, hasScript bool) SkillR } } -// AnalyzeSkillEntry scores a SkillEntry for security risk. -func AnalyzeSkillEntry(entry *contract.SkillEntry, hasScript bool) SkillRiskAssessment { +// AnalyzeSkillEntry scores a SkillEntry for security risk under the given +// policy. A zero-value SecurityPolicy{} preserves the historical default +// scoring (no overrides applied). +func AnalyzeSkillEntry(entry *contract.SkillEntry, hasScript bool, policy SecurityPolicy) SkillRiskAssessment { var factors []RiskFactor var egressDomains []string var bins []string @@ -97,9 +101,9 @@ func AnalyzeSkillEntry(entry *contract.SkillEntry, hasScript bool) SkillRiskAsse } } - factors = append(factors, scoreEgress(egressDomains, nil)...) - factors = append(factors, scoreBinaries(bins)...) - factors = append(factors, scoreEnv(reqEnv, oneOfEnv, optEnv)...) + factors = append(factors, scoreEgress(egressDomains, policy)...) + factors = append(factors, scoreBinaries(bins, policy)...) + factors = append(factors, scoreEnv(reqEnv, oneOfEnv, optEnv, policy)...) if hasScript { factors = append(factors, scoreScript()...) } @@ -113,25 +117,32 @@ func AnalyzeSkillEntry(entry *contract.SkillEntry, hasScript bool) SkillRiskAsse } } -func scoreEgress(domains []string, extraTrusted []string) []RiskFactor { +func scoreEgress(domains []string, policy SecurityPolicy) []RiskFactor { var factors []RiskFactor - trusted := make(map[string]bool) - for k, v := range trustedDomains { - trusted[k] = v - } - for _, d := range extraTrusted { - trusted[d] = true + // Union builtin trusted domains with policy.TrustedDomains. We track the + // policy-trusted set separately so the RiskFactor description can record + // that the trust came from a policy override. + policyTrusted := make(map[string]bool, len(policy.TrustedDomains)) + for _, d := range policy.TrustedDomains { + policyTrusted[d] = true } for _, domain := range domains { - if trusted[domain] { + switch { + case trustedDomains[domain]: factors = append(factors, RiskFactor{ Category: "egress", Description: fmt.Sprintf("trusted domain: %s", domain), Points: 2, }) - } else { + case policyTrusted[domain]: + factors = append(factors, RiskFactor{ + Category: "egress", + Description: fmt.Sprintf("trusted domain (via policy): %s", domain), + Points: 2, + }) + default: factors = append(factors, RiskFactor{ Category: "egress", Description: fmt.Sprintf("unknown domain: %s", domain), @@ -151,16 +162,28 @@ func scoreEgress(domains []string, extraTrusted []string) []RiskFactor { return factors } -func scoreBinaries(bins []string) []RiskFactor { +func scoreBinaries(bins []string, policy SecurityPolicy) []RiskFactor { + acknowledged := make(map[string]bool, len(policy.AcknowledgedBins)) + for _, b := range policy.AcknowledgedBins { + acknowledged[b] = true + } + var factors []RiskFactor for _, bin := range bins { - if highRiskBinaries[bin] { + switch { + case highRiskBinaries[bin] && acknowledged[bin]: + factors = append(factors, RiskFactor{ + Category: "binary", + Description: fmt.Sprintf("high-risk binary (acknowledged by policy): %s", bin), + Points: 3, + }) + case highRiskBinaries[bin]: factors = append(factors, RiskFactor{ Category: "binary", Description: fmt.Sprintf("high-risk binary: %s", bin), Points: 15, }) - } else { + default: factors = append(factors, RiskFactor{ Category: "binary", Description: fmt.Sprintf("standard binary: %s", bin), @@ -171,21 +194,33 @@ func scoreBinaries(bins []string) []RiskFactor { return factors } -func scoreEnv(reqEnv, oneOfEnv, optEnv []string) []RiskFactor { - var factors []RiskFactor +func scoreEnv(reqEnv, oneOfEnv, optEnv []string, policy SecurityPolicy) []RiskFactor { + acknowledged := make(map[string]bool, len(policy.AcknowledgedEnv)) + for _, e := range policy.AcknowledgedEnv { + acknowledged[e] = true + } + allEnv := make([]string, 0, len(reqEnv)+len(oneOfEnv)+len(optEnv)) allEnv = append(allEnv, reqEnv...) allEnv = append(allEnv, oneOfEnv...) allEnv = append(allEnv, optEnv...) + var factors []RiskFactor for _, env := range allEnv { - if isSensitiveEnv(env) { + switch { + case isSensitiveEnv(env) && acknowledged[env]: + factors = append(factors, RiskFactor{ + Category: "env", + Description: fmt.Sprintf("sensitive variable (acknowledged by policy): %s", env), + Points: 5, + }) + case isSensitiveEnv(env): factors = append(factors, RiskFactor{ Category: "env", Description: fmt.Sprintf("sensitive variable: %s", env), Points: 10, }) - } else { + default: factors = append(factors, RiskFactor{ Category: "env", Description: fmt.Sprintf("API key: %s", env), diff --git a/forge-skills/analyzer/scoring_test.go b/forge-skills/analyzer/scoring_test.go index 787aa9d..5a88946 100644 --- a/forge-skills/analyzer/scoring_test.go +++ b/forge-skills/analyzer/scoring_test.go @@ -1,14 +1,21 @@ package analyzer import ( + "strings" "testing" "github.com/initializ/forge/forge-skills/contract" ) +// contains is a thin wrapper around strings.Contains used by override tests +// to make intent-revealing assertions on factor descriptions. +func contains(haystack, needle string) bool { + return strings.Contains(haystack, needle) +} + func TestAnalyzeSkillDescriptor_NoRisk(t *testing.T) { sd := &contract.SkillDescriptor{Name: "simple"} - a := AnalyzeSkillDescriptor(sd, false) + a := AnalyzeSkillDescriptor(sd, false, SecurityPolicy{}) if a.Score.Value != 0 { t.Fatalf("expected score 0, got %d", a.Score.Value) @@ -23,7 +30,7 @@ func TestAnalyzeSkillDescriptor_TrustedDomain(t *testing.T) { Name: "github", EgressDomains: []string{"api.github.com"}, } - a := AnalyzeSkillDescriptor(sd, false) + a := AnalyzeSkillDescriptor(sd, false, SecurityPolicy{}) if a.Score.Value != 2 { t.Fatalf("expected score 2, got %d", a.Score.Value) @@ -35,7 +42,7 @@ func TestAnalyzeSkillDescriptor_UnknownDomain(t *testing.T) { Name: "custom", EgressDomains: []string{"evil.example.com"}, } - a := AnalyzeSkillDescriptor(sd, false) + a := AnalyzeSkillDescriptor(sd, false, SecurityPolicy{}) if a.Score.Value != 10 { t.Fatalf("expected score 10, got %d", a.Score.Value) @@ -47,7 +54,7 @@ func TestAnalyzeSkillDescriptor_HighRiskBinary(t *testing.T) { Name: "shell-tool", RequiredBins: []string{"bash"}, } - a := AnalyzeSkillDescriptor(sd, false) + a := AnalyzeSkillDescriptor(sd, false, SecurityPolicy{}) if a.Score.Value != 15 { t.Fatalf("expected score 15, got %d", a.Score.Value) @@ -59,7 +66,7 @@ func TestAnalyzeSkillDescriptor_StandardBinary(t *testing.T) { Name: "api-tool", RequiredBins: []string{"curl"}, } - a := AnalyzeSkillDescriptor(sd, false) + a := AnalyzeSkillDescriptor(sd, false, SecurityPolicy{}) if a.Score.Value != 3 { t.Fatalf("expected score 3, got %d", a.Score.Value) @@ -71,7 +78,7 @@ func TestAnalyzeSkillDescriptor_SensitiveEnv(t *testing.T) { Name: "secret-tool", RequiredEnv: []string{"AWS_SECRET_ACCESS_KEY"}, } - a := AnalyzeSkillDescriptor(sd, false) + a := AnalyzeSkillDescriptor(sd, false, SecurityPolicy{}) if a.Score.Value != 10 { t.Fatalf("expected score 10, got %d", a.Score.Value) @@ -83,7 +90,7 @@ func TestAnalyzeSkillDescriptor_StandardAPIKey(t *testing.T) { Name: "api-tool", RequiredEnv: []string{"GH_TOKEN"}, } - a := AnalyzeSkillDescriptor(sd, false) + a := AnalyzeSkillDescriptor(sd, false, SecurityPolicy{}) if a.Score.Value != 5 { t.Fatalf("expected score 5, got %d", a.Score.Value) @@ -92,7 +99,7 @@ func TestAnalyzeSkillDescriptor_StandardAPIKey(t *testing.T) { func TestAnalyzeSkillDescriptor_WithScript(t *testing.T) { sd := &contract.SkillDescriptor{Name: "scripted"} - a := AnalyzeSkillDescriptor(sd, true) + a := AnalyzeSkillDescriptor(sd, true, SecurityPolicy{}) if a.Score.Value != 20 { t.Fatalf("expected score 20, got %d", a.Score.Value) @@ -106,7 +113,7 @@ func TestAnalyzeSkillDescriptor_Combined(t *testing.T) { RequiredBins: []string{"gh"}, RequiredEnv: []string{"GH_TOKEN"}, } - a := AnalyzeSkillDescriptor(sd, false) + a := AnalyzeSkillDescriptor(sd, false, SecurityPolicy{}) // 2 + 2 + 3 + 5 = 12 expected := 12 @@ -126,7 +133,7 @@ func TestAnalyzeSkillDescriptor_CappedAt100(t *testing.T) { RequiredBins: []string{"bash", "python", "ssh", "nc"}, RequiredEnv: []string{"AWS_SECRET_ACCESS_KEY", "DB_PASSWORD"}, } - a := AnalyzeSkillDescriptor(sd, true) + a := AnalyzeSkillDescriptor(sd, true, SecurityPolicy{}) if a.Score.Value > 100 { t.Fatalf("score should be capped at 100, got %d", a.Score.Value) @@ -139,7 +146,7 @@ func TestAnalyzeSkillDescriptor_ManyDomainBonus(t *testing.T) { Name: "many-domains", EgressDomains: domains, } - a := AnalyzeSkillDescriptor(sd, false) + a := AnalyzeSkillDescriptor(sd, false, SecurityPolicy{}) // 6 unknown domains * 10 = 60, plus bonus 15 = 75 if a.Score.Value != 75 { @@ -170,6 +177,108 @@ func TestClassifyScore(t *testing.T) { } } +// TestAnalyzeSkillDescriptor_PolicyTrustedDomain confirms a domain listed in +// SecurityPolicy.TrustedDomains is scored as trusted (+2) and the factor +// description marks the override as "via policy" so the audit report is +// auditable. +func TestAnalyzeSkillDescriptor_PolicyTrustedDomain(t *testing.T) { + sd := &contract.SkillDescriptor{ + Name: "internal-tool", + EgressDomains: []string{"internal.example.com"}, + } + policy := SecurityPolicy{TrustedDomains: []string{"internal.example.com"}} + a := AnalyzeSkillDescriptor(sd, false, policy) + + if a.Score.Value != 2 { + t.Fatalf("expected score 2 with TrustedDomains override, got %d", a.Score.Value) + } + if len(a.Factors) != 1 { + t.Fatalf("expected 1 factor, got %d", len(a.Factors)) + } + if !contains(a.Factors[0].Description, "via policy") { + t.Errorf("expected factor description to mark policy override, got %q", a.Factors[0].Description) + } +} + +// TestAnalyzeSkillDescriptor_PolicyAcknowledgedBin confirms a high-risk +// binary listed in SecurityPolicy.AcknowledgedBins drops from +15 to +3 and +// the factor description records that the override was applied. +func TestAnalyzeSkillDescriptor_PolicyAcknowledgedBin(t *testing.T) { + sd := &contract.SkillDescriptor{ + Name: "scripted-tool", + RequiredBins: []string{"python"}, + } + policy := SecurityPolicy{AcknowledgedBins: []string{"python"}} + a := AnalyzeSkillDescriptor(sd, false, policy) + + if a.Score.Value != 3 { + t.Fatalf("expected score 3 with AcknowledgedBins override, got %d", a.Score.Value) + } + if !contains(a.Factors[0].Description, "acknowledged by policy") { + t.Errorf("expected factor description to mark policy override, got %q", a.Factors[0].Description) + } +} + +// TestAnalyzeSkillDescriptor_PolicyAcknowledgedEnv confirms an env var that +// matches a builtin sensitive pattern drops from +10 to +5 when listed in +// SecurityPolicy.AcknowledgedEnv, with the override recorded. +func TestAnalyzeSkillDescriptor_PolicyAcknowledgedEnv(t *testing.T) { + sd := &contract.SkillDescriptor{ + Name: "db-tool", + RequiredEnv: []string{"DB_PASSWORD"}, + } + policy := SecurityPolicy{AcknowledgedEnv: []string{"DB_PASSWORD"}} + a := AnalyzeSkillDescriptor(sd, false, policy) + + if a.Score.Value != 5 { + t.Fatalf("expected score 5 with AcknowledgedEnv override, got %d", a.Score.Value) + } + if !contains(a.Factors[0].Description, "acknowledged by policy") { + t.Errorf("expected factor description to mark policy override, got %q", a.Factors[0].Description) + } +} + +// TestAnalyzeSkillDescriptor_PolicyPreservesDefault confirms that supplying +// an empty policy alongside an item already covered by the builtin tables +// keeps the historical score (no double-trust, no description change). +func TestAnalyzeSkillDescriptor_PolicyPreservesDefault(t *testing.T) { + sd := &contract.SkillDescriptor{ + Name: "github", + EgressDomains: []string{"api.github.com"}, + } + // Listing a builtin-trusted domain in policy must not change the factor + // description — builtin classification wins so the audit report stays + // consistent across projects that share the same builtin allowlist. + policy := SecurityPolicy{TrustedDomains: []string{"api.github.com"}} + a := AnalyzeSkillDescriptor(sd, false, policy) + + if a.Score.Value != 2 { + t.Fatalf("expected score 2, got %d", a.Score.Value) + } + if contains(a.Factors[0].Description, "via policy") { + t.Errorf("builtin trust should not be re-attributed to policy, got %q", a.Factors[0].Description) + } +} + +// TestAnalyzeSkillDescriptor_PolicyDoesNotPromoteStandardBin confirms that +// listing a binary in AcknowledgedBins has no effect if the binary is not +// in the builtin highRiskBinaries set — the score stays at the standard +3. +func TestAnalyzeSkillDescriptor_PolicyDoesNotPromoteStandardBin(t *testing.T) { + sd := &contract.SkillDescriptor{ + Name: "curl-tool", + RequiredBins: []string{"curl"}, + } + policy := SecurityPolicy{AcknowledgedBins: []string{"curl"}} + a := AnalyzeSkillDescriptor(sd, false, policy) + + if a.Score.Value != 3 { + t.Fatalf("expected score 3 (standard bin), got %d", a.Score.Value) + } + if contains(a.Factors[0].Description, "acknowledged") { + t.Errorf("standard binary should not be annotated as acknowledged, got %q", a.Factors[0].Description) + } +} + func TestGenerateRecommendations(t *testing.T) { factors := []RiskFactor{ {Category: "binary", Points: 15}, diff --git a/forge-skills/analyzer/types.go b/forge-skills/analyzer/types.go index 06948e3..a371feb 100644 --- a/forge-skills/analyzer/types.go +++ b/forge-skills/analyzer/types.go @@ -60,12 +60,23 @@ type PolicySummary struct { } // SecurityPolicy defines configurable security rules. +// +// Policy-check fields raise PolicyViolations during CheckPolicy. Scoring +// override fields influence how AnalyzeSkill* assigns points — they reduce +// the score for items an operator has explicitly accepted, and the affected +// RiskFactor's Description is annotated with "(via policy)" so the override +// is visible in the audit report. type SecurityPolicy struct { + // Policy checks. MaxEgressDomains int `yaml:"max_egress_domains" json:"max_egress_domains"` BinaryDenylist []string `yaml:"binary_denylist" json:"binary_denylist,omitempty"` DeniedEnvPatterns []string `yaml:"denied_env_patterns" json:"denied_env_patterns,omitempty"` ScriptPolicy string `yaml:"script_policy" json:"script_policy"` // "allow"|"warn"|"deny" MaxRiskScore int `yaml:"max_risk_score" json:"max_risk_score"` MaxTags int `yaml:"max_tags" json:"max_tags"` - TrustedDomains []string `yaml:"trusted_domains" json:"trusted_domains,omitempty"` + + // Scoring overrides. + TrustedDomains []string `yaml:"trusted_domains" json:"trusted_domains,omitempty"` // egress domains scored as trusted (+2) instead of unknown (+10) + AcknowledgedBins []string `yaml:"acknowledged_bins" json:"acknowledged_bins,omitempty"` // builtin high-risk binaries scored as standard (+3) instead of high-risk (+15) + AcknowledgedEnv []string `yaml:"acknowledged_env" json:"acknowledged_env,omitempty"` // env vars matching builtin sensitive patterns scored as standard (+5) instead of sensitive (+10) }