diff --git a/.plumber.yaml b/.plumber.yaml index 54d4d379..75059df0 100644 --- a/.plumber.yaml +++ b/.plumber.yaml @@ -178,6 +178,15 @@ gitlab: # Minimum access level required to push (0=No one, 30=Developer, 40=Maintainer) minPushAccessLevel: 40 # =========================================== + # Project must have a security policy source + # =========================================== + # Requires the project to link a GitLab security policy project (Settings > + # Security & Compliance > Policies). Set expectedProjectId (numeric) OR + # expectedProjectPath (full path, case-insensitive) to require a specific + # policy project — the ID wins if both are set; leave both unset to require + # only that SOME policy project is linked. REQUIRES GITLAB ULTIMATE. + projectMustHaveSecurityPolicySource: + # =========================================== # Pipeline must not include hardcoded jobs # =========================================== # Detects CI/CD jobs that are defined directly in the .gitlab-ci.yml file diff --git a/cmd/analyze_shared.go b/cmd/analyze_shared.go index a9c885b7..6821c3ca 100644 --- a/cmd/analyze_shared.go +++ b/cmd/analyze_shared.go @@ -104,6 +104,7 @@ func outputTextWithProvider(p provider.Provider, result *control.AnalysisResult, // real) and drop the green stat blocks (#220). renderFindingGroups(filterGroupsForDegraded(groups, result.DataCollectionDegraded)) renderWarnings(result.Warnings) + renderSecurityPolicyTierCaveat(result) printSectionHeader("Summary") fmt.Println() diff --git a/cmd/legacy_json.go b/cmd/legacy_json.go index 0489987f..90317345 100644 --- a/cmd/legacy_json.go +++ b/cmd/legacy_json.go @@ -98,10 +98,43 @@ func _withControlMeta(block any, e control.ControlEntry, result *control.Analysi if m, ok := block.(map[string]any); ok { m["controlName"] = e.ControlName m["status"] = control.StatusFor(e, result, findingCount) + if result != nil && result.SecurityPolicyTierCaveat && e.ControlName == "projectMustHaveSecurityPolicySource" { + // No policy project is linked, and security policies are an Ultimate + // feature, so we cannot tell a non-Ultimate project (unable to link) + // from an Ultimate project that left it unset. Keyed so a consumer can + // annotate ISSUE-601. + m["tierCaveat"] = map[string]any{ + "reason": "no-security-policy-project-linked", + "requiresTier": "ultimate", + "message": securityPolicyTierCaveatMessage, + } + } } return block } +// securityPolicyTierCaveatMessage explains the Ultimate requirement for +// ISSUE-601 when no security policy project is linked. Shared by the terminal +// caveat (render_details.go) and the JSON tierCaveat. +const securityPolicyTierCaveatMessage = "Security policies require GitLab Ultimate, and no policy project is linked. If this project is not on GitLab Ultimate it cannot link one — disable this control. If it is, link the expected security policy project to satisfy the check." + +// buildSecurityPolicyProjectBlock emits the legacy JSON block for the +// security-policy-project linkage control (ISSUE-601). The finding is a +// project-level singleton (no file/job); its linkedProjectId / linkedProjectPath +// / expectedProjectId ride in the issue's data, preserved by projectFindings. +func buildSecurityPolicyProjectBlock(c legacyCommon, findings []opaengine.Finding) map[string]any { + return map[string]any{ + "issues": projectFindings(findings, "job"), + "metrics": map[string]any{ + "projectWithoutExpectedSecurityPolicy": len(findings), + }, + "version": "0.1.0", + "ciValid": c.CiValid, + "ciMissing": c.CiMissing, + "skipped": c.Skipped, + } +} + // buildLegacyResult routes a control entry to its legacy JSON // builder and returns the (jsonKey, block) pair. func buildLegacyResult(e control.ControlEntry, result *control.AnalysisResult, pc *configuration.PlumberConfig, findings []opaengine.Finding) (string, any) { @@ -118,6 +151,8 @@ func buildLegacyResult(e control.ControlEntry, result *control.AnalysisResult, p return "imageAuthorizedSourcesResult", buildImageAuthorizedSourcesBlock(common, result, findings) case "branchMustBeProtected": return "branchProtectionResult", buildBranchProtectionBlock(common, result, pc, findings) + case "projectMustHaveSecurityPolicySource": + return "securityPolicyProjectResult", buildSecurityPolicyProjectBlock(common, findings) case "pipelineMustNotIncludeHardcodedJobs": return "hardcodedJobsResult", buildHardcodedJobsBlock(common, result, findings) case "externalRefsMustNotCollide": diff --git a/cmd/render_details.go b/cmd/render_details.go index 6c7e48a6..b2208abb 100644 --- a/cmd/render_details.go +++ b/cmd/render_details.go @@ -867,10 +867,28 @@ func buildGitLabControlStats(controlName string, result *control.AnalysisResult, {Label: "Unprotected", Value: fmt.Sprintf("%d", unprotected)}, {Label: "Non-Compliant", Value: fmt.Sprintf("%d", nonCompliant)}, } + case "projectMustHaveSecurityPolicySource": + return []statLine{ + {Label: "Security Policy Not Linked", Value: fmt.Sprintf("%d", findingsCount)}, + } } return nil } +// renderSecurityPolicyTierCaveat prints a caveat when the security-policy +// control flagged a project with no policy project linked: the feature requires +// GitLab Ultimate, and a non-Ultimate project cannot link one, so the failure +// may be a tier limitation rather than a real misconfiguration (see +// AnalysisResult.SecurityPolicyTierCaveat). No-op otherwise. +func renderSecurityPolicyTierCaveat(result *control.AnalysisResult) { + if result == nil || !result.SecurityPolicyTierCaveat { + return + } + fmt.Println() + fmt.Printf(" %s⚠ Security policies are a GitLab Ultimate feature, and no policy project is linked.%s\n", colorYellow, colorReset) + fmt.Printf(" %s•%s If this project isn't on GitLab Ultimate it can't link one, so disable this control. If it is, link the expected security policy project to satisfy the check.\n", colorYellow, colorReset) +} + // _countScriptLines walks the merged GitLab CI conf and totals every // script line declared on every job (script, before_script, // after_script). Used as the "Script Lines Checked" denominator diff --git a/configuration/plumberconfig.go b/configuration/plumberconfig.go index bbaaaefe..4150aaaa 100644 --- a/configuration/plumberconfig.go +++ b/configuration/plumberconfig.go @@ -36,6 +36,9 @@ var validControlSchema = map[string][]string{ "allowForcePush", "codeOwnerApprovalRequired", "minMergeAccessLevel", "minPushAccessLevel", }, + "projectMustHaveSecurityPolicySource": { + "enabled", "expectedProjectId", "expectedProjectPath", + }, "pipelineMustNotIncludeHardcodedJobs": { "enabled", }, @@ -258,6 +261,11 @@ type ControlsConfig struct { // BranchMustBeProtected control configuration BranchMustBeProtected *BranchProtectionControlConfig `yaml:"branchMustBeProtected,omitempty"` + // ProjectMustHaveSecurityPolicySource control configuration (GitLab only). + // Requires the project to link the expected GitLab security policy project + // (ISSUE-601). Requires GitLab Ultimate. + ProjectMustHaveSecurityPolicySource *SecurityPolicyControlConfig `yaml:"projectMustHaveSecurityPolicySource,omitempty"` + // PipelineMustNotIncludeHardcodedJobs control configuration PipelineMustNotIncludeHardcodedJobs *HardcodedJobsControlConfig `yaml:"pipelineMustNotIncludeHardcodedJobs,omitempty"` @@ -405,6 +413,37 @@ type EnabledOnlyControlConfig struct { Enabled *bool `yaml:"enabled,omitempty"` } +// SecurityPolicyControlConfig configures the GitLab security-policy-project +// linkage check (ISSUE-601). GitLab-only, requires Ultimate. When +// ExpectedProjectId is set, the linked policy project must be exactly that +// project. When it is unset, any linked policy project passes and the control +// fails only when none is linked. +type SecurityPolicyControlConfig struct { + // Enabled controls whether this check runs. + Enabled *bool `yaml:"enabled,omitempty"` + + // ExpectedProjectId is the numeric GitLab project ID the security policy + // project must match. Unset => require only that some policy project is + // linked. + ExpectedProjectId *int `yaml:"expectedProjectId,omitempty"` + + // ExpectedProjectPath is the full path (namespace/project) the linked + // security policy project must match — a human-friendly alternative to the + // numeric ID, compared case-insensitively. Ignored when ExpectedProjectId is + // also set (the ID is authoritative). Unset (and no ID) => require only that + // some policy project is linked. + ExpectedProjectPath *string `yaml:"expectedProjectPath,omitempty"` +} + +// IsEnabled reports whether the control is enabled. Returns false when the +// wrapper or the field is nil — same convention as every other IsEnabled(). +func (c *SecurityPolicyControlConfig) IsEnabled() bool { + if c == nil || c.Enabled == nil { + return false + } + return *c.Enabled +} + // IsEnabled reports whether the control is enabled. Returns false when // the wrapper or the field is nil — same convention as every other // IsEnabled() in this package. @@ -1131,6 +1170,15 @@ func (c *PlumberConfig) GetBranchMustBeProtectedConfig() *BranchProtectionContro return c.ControlsFor("gitlab").BranchMustBeProtected } +// GetProjectMustHaveSecurityPolicySourceConfig returns the GitLab +// security-policy-project linkage control configuration (ISSUE-601), or nil. +func (c *PlumberConfig) GetProjectMustHaveSecurityPolicySourceConfig() *SecurityPolicyControlConfig { + if c == nil { + return nil + } + return c.ControlsFor("gitlab").ProjectMustHaveSecurityPolicySource +} + // IsEnabled returns whether the control is enabled // Returns false if not properly configured func (c *BranchProtectionControlConfig) IsEnabled() bool { diff --git a/configuration/plumberconfig_test.go b/configuration/plumberconfig_test.go index 42700797..a2c3f4ef 100644 --- a/configuration/plumberconfig_test.go +++ b/configuration/plumberconfig_test.go @@ -375,6 +375,7 @@ func TestValidControlNames(t *testing.T) { "pipelineMustNotOverrideJobVariables", "pipelineMustNotUseDockerInDocker", "pipelineMustNotUseUnsafeVariableExpansion", + "projectMustHaveSecurityPolicySource", "pullRequestTargetMustNotCheckoutHead", "releaseWorkflowsMustNotRestoreUntrustedCache", "reusableWorkflowsMustNotInheritSecrets", diff --git a/configuration/registry.go b/configuration/registry.go index 888043f1..ba41fb5e 100644 --- a/configuration/registry.go +++ b/configuration/registry.go @@ -31,6 +31,7 @@ const ( var controlsMeta = map[string]ControlMeta{ // Cross-provider (same control name + rego logic, provider-specific values). "branchMustBeProtected": {Providers: []string{ProviderGitLab, ProviderGitHub}}, + "projectMustHaveSecurityPolicySource": {Providers: []string{ProviderGitLab}}, "containerImageMustComeFromAuthorizedSources": {Providers: []string{ProviderGitLab, ProviderGitHub}}, "containerImageMustNotUseForbiddenTags": {Providers: []string{ProviderGitLab, ProviderGitHub}}, "externalRefsMustNotCollide": {Providers: []string{ProviderGitLab, ProviderGitHub}}, diff --git a/configuration/v1_to_v2.go b/configuration/v1_to_v2.go index 737b6be1..2859da24 100644 --- a/configuration/v1_to_v2.go +++ b/configuration/v1_to_v2.go @@ -71,6 +71,7 @@ func controlsConfigIsZero(c ControlsConfig) bool { return c.ContainerImageMustNotUseForbiddenTags == nil && c.ContainerImageMustComeFromAuthorizedSources == nil && c.BranchMustBeProtected == nil && + c.ProjectMustHaveSecurityPolicySource == nil && c.PipelineMustNotIncludeHardcodedJobs == nil && c.IncludesMustBeUpToDate == nil && c.IncludesMustNotUseForbiddenVersions == nil && @@ -96,6 +97,7 @@ func controlsConfigEqual(a, b ControlsConfig) bool { return a.ContainerImageMustNotUseForbiddenTags == b.ContainerImageMustNotUseForbiddenTags && a.ContainerImageMustComeFromAuthorizedSources == b.ContainerImageMustComeFromAuthorizedSources && a.BranchMustBeProtected == b.BranchMustBeProtected && + a.ProjectMustHaveSecurityPolicySource == b.ProjectMustHaveSecurityPolicySource && a.PipelineMustNotIncludeHardcodedJobs == b.PipelineMustNotIncludeHardcodedJobs && a.IncludesMustBeUpToDate == b.IncludesMustBeUpToDate && a.IncludesMustNotUseForbiddenVersions == b.IncludesMustNotUseForbiddenVersions && diff --git a/control/catalog.go b/control/catalog.go index 67d8c6fe..a47a800d 100644 --- a/control/catalog.go +++ b/control/catalog.go @@ -52,6 +52,11 @@ func GitLabControls(pc *configuration.PlumberConfig) []ControlEntry { ControlName: "branchMustBeProtected", Skipped: c.BranchMustBeProtected == nil || !c.BranchMustBeProtected.IsEnabled(), }) + entries = append(entries, ControlEntry{ + DisplayName: "Project must have a security policy source", + ControlName: "projectMustHaveSecurityPolicySource", + Skipped: c.ProjectMustHaveSecurityPolicySource == nil || !c.ProjectMustHaveSecurityPolicySource.IsEnabled(), + }) entries = append(entries, ControlEntry{ DisplayName: "Pipeline must not include hardcoded jobs", ControlName: "pipelineMustNotIncludeHardcodedJobs", @@ -315,6 +320,9 @@ func DisabledControlNames(c *configuration.ControlsConfig) map[string]bool { if cfg := c.BranchMustBeProtected; cfg == nil || !cfg.IsEnabled() { out["branchMustBeProtected"] = true } + if cfg := c.ProjectMustHaveSecurityPolicySource; cfg == nil || !cfg.IsEnabled() { + out["projectMustHaveSecurityPolicySource"] = true + } if cfg := c.PipelineMustNotIncludeHardcodedJobs; cfg == nil || !cfg.IsEnabled() { out["pipelineMustNotIncludeHardcodedJobs"] = true } diff --git a/control/codes.go b/control/codes.go index d3dabe56..386857b1 100644 --- a/control/codes.go +++ b/control/codes.go @@ -152,8 +152,8 @@ const ( // Issue codes for workflow-hygiene controls (6xx) const ( - // ISSUE-601: Workflow has no explicit `name:` field - CodeAnonymousDefinition ErrorCode = "ISSUE-601" + // ISSUE-422: Workflow has no explicit `name:` field + CodeAnonymousDefinition ErrorCode = "ISSUE-422" // ISSUE-418: Workflow has no `concurrency:` block at either level CodeMissingConcurrency ErrorCode = "ISSUE-418" // ISSUE-419: Workflow uses a misfeature pattern (shell: cmd, inline pip install curl|sh, …) @@ -180,6 +180,8 @@ const ( CodeBranchUnprotected ErrorCode = "ISSUE-501" // ISSUE-505: Branch has non-compliant protection settings CodeBranchNonCompliant ErrorCode = "ISSUE-505" + // ISSUE-601: No (or the wrong) GitLab security policy project is linked + CodeSecurityPolicyProjectNotSet ErrorCode = "ISSUE-601" // ISSUE-803: Job runs with overly broad permissions (write-all) CodeExcessivePermissions ErrorCode = "ISSUE-803" ) @@ -559,6 +561,15 @@ var errorCodeRegistry = map[ErrorCode]ErrorCodeInfo{ DocURL: docsBaseURL + string(CodeBranchNonCompliant), ControlName: "branchMustBeProtected", }, + CodeSecurityPolicyProjectNotSet: { + Code: CodeSecurityPolicyProjectNotSet, + Severity: SeverityCritical, + Title: "Missing security policy source on project", + Description: "The project does not directly link the expected GitLab security policy project (none is linked, or a different one than the configured expectation). This checks the project's own link only, so a security policy source inherited from a parent group is not detected.", + Remediation: "Link the expected security policy project in Settings > Security & Compliance > Policies (or set it via the API). If your policies are enforced at a parent group and inherited, this project-scoped check will not see them, so link at the project level too or disable this control. Security policies require GitLab Ultimate.", + DocURL: docsBaseURL + string(CodeSecurityPolicyProjectNotSet), + ControlName: "projectMustHaveSecurityPolicySource", + }, CodeTemplateInjection: { Code: CodeTemplateInjection, Severity: SeverityCritical, diff --git a/control/status.go b/control/status.go index bcf0b01e..9376640a 100644 --- a/control/status.go +++ b/control/status.go @@ -88,6 +88,16 @@ func StatusFor(e ControlEntry, result *AnalysisResult, findingCount int) string } return StatusPassed } + if e.ControlName == "projectMustHaveSecurityPolicySource" { + // Reached only with zero findings (a finding returned Failed above). The + // linkage is read over its own API surface; when it could not be read + // authoritatively (401/403 or the field is unavailable) the control never + // truly evaluated and must not read as a pass. + if result.SecurityPolicyEvaluable { + return StatusPassed + } + return StatusError + } if result.CiMissing || !result.CiValid { return StatusError } diff --git a/control/task.go b/control/task.go index e1035120..e207542b 100644 --- a/control/task.go +++ b/control/task.go @@ -33,6 +33,45 @@ const opaEvaluateTimeout = 2 * time.Minute // the catalog in catalog.go. const controlBranchMustBeProtected = "branchMustBeProtected" const controlMutableRemoteExec = "actionsMustNotExecuteMutableRemoteCode" +const controlSecurityPolicy = "projectMustHaveSecurityPolicySource" + +// securityPolicyControlEnabled reports whether the security-policy-project +// linkage control (ISSUE-601) is active for this run. It reads the linkage the +// GitLab protection collection fetches, so that collection must run when it is +// enabled even if branchMustBeProtected is not. +func securityPolicyControlEnabled(conf *configuration.Configuration) bool { + if conf == nil || conf.PlumberConfig == nil { + return false + } + c := conf.PlumberConfig.GetProjectMustHaveSecurityPolicySourceConfig() + return c != nil && c.IsEnabled() && shouldRunControl(controlSecurityPolicy, conf) +} + +// protectionDataNeeded reports whether any control needs the GitLab protection +// collection this run: branchMustBeProtected or the security-policy control +// (they read the one GitlabProtectionAnalysisData). +func protectionDataNeeded(conf *configuration.Configuration) bool { + if shouldRunControl(controlBranchMustBeProtected, conf) { + if cfg := conf.PlumberConfig.GetBranchMustBeProtectedConfig(); cfg != nil && cfg.IsEnabled() { + return true + } + } + return securityPolicyControlEnabled(conf) +} + +// securityPolicyTierCaveatApplies reports whether to surface the conditional +// Ultimate caveat for ISSUE-601: the control ran, the linkage was read +// authoritatively, and NO policy project is linked — the tier-ambiguous case +// (a non-Ultimate project cannot link one, but an Ultimate project may simply +// have left it unset). A wrong-project-linked read is a real misconfiguration +// on a paid tier, not a tier caveat, and a non-authoritative read is +// not-evaluable, so neither triggers it. +func securityPolicyTierCaveatApplies(conf *configuration.Configuration, protectionData *gitlab.GitlabProtectionAnalysisData) bool { + if !securityPolicyControlEnabled(conf) || protectionData == nil || !protectionData.SecurityPolicyKnown { + return false + } + return protectionData.SecurityPolicyProject == nil +} // shouldScanMutableExec reports whether the collector should fetch and // scan action source for actionsMustNotExecuteMutableRemoteCode @@ -178,6 +217,20 @@ func buildEngineConfig(controls *configuration.ControlsConfig) map[string]any { } cfg := map[string]any{} + if c := controls.ProjectMustHaveSecurityPolicySource; c != nil { + // expectedProjectId / expectedProjectPath reach the engine only when set: + // the Rego rule treats their absence as "require any linkage", the id as + // the authoritative match, and the path as a case-insensitive fallback. + entry := map[string]any{} + if c.ExpectedProjectId != nil { + entry["expectedProjectId"] = *c.ExpectedProjectId + } + if c.ExpectedProjectPath != nil { + entry["expectedProjectPath"] = *c.ExpectedProjectPath + } + cfg["projectMustHaveSecurityPolicySource"] = entry + } + if c := controls.ContainerImageMustNotUseForbiddenTags; c != nil { if len(c.Tags) > 0 { cfg["imageMutableTag"] = map[string]any{ @@ -626,8 +679,8 @@ func RunAnalysis(conf *configuration.Configuration) (*AnalysisResult, error) { // corresponding control — the Rego policy needs the protection // settings to check every branch against the declared bar. var protectionData *gitlab.GitlabProtectionAnalysisData - if shouldRunControl(controlBranchMustBeProtected, conf) { - if cfg := conf.PlumberConfig.GetBranchMustBeProtectedConfig(); cfg != nil && cfg.IsEnabled() { + if protectionDataNeeded(conf) { + { reportProgress(conf, 9, analysisStepCount, "Checking branch protection") protectionDC := &gitlab.GitlabProtectionDataCollection{} pData, _, pErr := protectionDC.Run(projectInfo, conf.GitlabToken, conf) @@ -651,6 +704,12 @@ func RunAnalysis(conf *configuration.Configuration) (*AnalysisResult, error) { // docs/REFACTOR_MULTI_PROVIDER.md §8 Phase A). result.Findings = runRegoEngine(l, conf, project, pipelineOriginData, pipelineImageData, protectionData) result.ProtectionData = protectionData + // ISSUE-601 is not-evaluable when the linkage could not be read (auth error / + // field unavailable): the collector leaves SecurityPolicyKnown false, so + // StatusFor reports error rather than a false pass. When it WAS read but + // nothing is linked, surface the conditional Ultimate tier caveat. + result.SecurityPolicyEvaluable = protectionData != nil && protectionData.SecurityPolicyKnown + result.SecurityPolicyTierCaveat = securityPolicyTierCaveatApplies(conf, protectionData) reportProgress(conf, analysisStepCount, analysisStepCount, "Analysis complete") diff --git a/control/task_security_policy_test.go b/control/task_security_policy_test.go new file mode 100644 index 00000000..0adc327d --- /dev/null +++ b/control/task_security_policy_test.go @@ -0,0 +1,159 @@ +package control + +import ( + "context" + "testing" + + "github.com/getplumber/plumber/configuration" + "github.com/getplumber/plumber/gitlab" + opaengine "github.com/getplumber/plumber/internal/engine/opa" + "github.com/getplumber/plumber/internal/ir" + "github.com/getplumber/plumber/policies" +) + +func spBoolPtr(b bool) *bool { return &b } +func spIntPtr(i int) *int { return &i } +func spStrPtr(s string) *string { return &s } + +func spConf(c *configuration.SecurityPolicyControlConfig) *configuration.Configuration { + return &configuration.Configuration{PlumberConfig: &configuration.PlumberConfig{ + GitLab: &configuration.ProviderConfig{Controls: configuration.ControlsConfig{ + ProjectMustHaveSecurityPolicySource: c, + }}, + }} +} + +// securityPolicyControlEnabled gates the protection collection for a +// security-policy-only run: wrongly false means the linkage is never fetched +// and ISSUE-601 silently reports not-evaluable on every run. +func TestSecurityPolicyControlEnabled(t *testing.T) { + if securityPolicyControlEnabled(&configuration.Configuration{}) { + t.Fatal("expected false when PlumberConfig is nil") + } + if securityPolicyControlEnabled(spConf(nil)) { + t.Fatal("expected false when the control is not configured") + } + if securityPolicyControlEnabled(spConf(&configuration.SecurityPolicyControlConfig{Enabled: spBoolPtr(false)})) { + t.Fatal("expected false when disabled") + } + if !securityPolicyControlEnabled(spConf(&configuration.SecurityPolicyControlConfig{Enabled: spBoolPtr(true)})) { + t.Fatal("expected true when enabled") + } + skipped := spConf(&configuration.SecurityPolicyControlConfig{Enabled: spBoolPtr(true)}) + skipped.SkipControlsFilter = []string{controlSecurityPolicy} + if securityPolicyControlEnabled(skipped) { + t.Fatal("expected false when in --skip-controls") + } + + // protectionDataNeeded must be true for a security-policy-only run so the + // protection collection (which carries the linkage) actually runs. + if !protectionDataNeeded(spConf(&configuration.SecurityPolicyControlConfig{Enabled: spBoolPtr(true)})) { + t.Fatal("expected protectionDataNeeded true when only the security-policy control is enabled") + } +} + +// securityPolicyTierCaveatApplies composes the enabled gate with the +// linkage-read state: it fires only when the linkage was read and nothing is +// linked. A wrong-project read (a real misconfig on a paid tier) and a +// not-read state must not trigger it. +func TestSecurityPolicyTierCaveatApplies(t *testing.T) { + enabled := spConf(&configuration.SecurityPolicyControlConfig{Enabled: spBoolPtr(true)}) + disabled := spConf(&configuration.SecurityPolicyControlConfig{Enabled: spBoolPtr(false)}) + + noneLinked := &gitlab.GitlabProtectionAnalysisData{SecurityPolicyKnown: true, SecurityPolicyProject: nil} + linked := &gitlab.GitlabProtectionAnalysisData{SecurityPolicyKnown: true, SecurityPolicyProject: &gitlab.SecurityPolicyProjectLink{ID: 5}} + notRead := &gitlab.GitlabProtectionAnalysisData{SecurityPolicyKnown: false} + + if securityPolicyTierCaveatApplies(disabled, noneLinked) { + t.Fatal("caveat must NOT fire when the control is disabled") + } + if !securityPolicyTierCaveatApplies(enabled, noneLinked) { + t.Fatal("caveat must fire when enabled, read, and nothing linked") + } + if securityPolicyTierCaveatApplies(enabled, linked) { + t.Fatal("caveat must NOT fire when a project is linked (paid tier, real misconfig)") + } + if securityPolicyTierCaveatApplies(enabled, notRead) { + t.Fatal("caveat must NOT fire when the linkage was not read (not-evaluable)") + } + if securityPolicyTierCaveatApplies(enabled, nil) { + t.Fatal("caveat must NOT fire when there is no protection data") + } +} + +// TestSecurityPolicyConfigContract pins the struct -> map -> rego chain for +// ISSUE-601: buildEngineConfig emits expectedProjectId only when set, and the +// rego reads exactly that key, so a rename on either side would silently make +// the control assert only "any linkage" forever. +func TestSecurityPolicyConfigContract(t *testing.T) { + engine := opaengine.New() + if err := engine.LoadFromFSFiltered(policies.FS, nil); err != nil { + t.Fatalf("load policies: %v", err) + } + fires := func(linkedID int, cfg map[string]any) bool { + p := &ir.NormalizedPipeline{Provider: ir.ProviderGitLab, SecurityPolicyProject: &ir.SecurityPolicyProjectState{Known: true, LinkedProjectID: linkedID}} + findings, err := engine.Evaluate(context.Background(), p, cfg) + if err != nil { + t.Fatalf("evaluate: %v", err) + } + for _, f := range findings { + if f.Code == "ISSUE-601" { + return true + } + } + return false + } + + // expectedProjectId set via the REAL projection: a mismatch fires, a match does not. + cfgExpect := buildEngineConfig(&configuration.ControlsConfig{ + ProjectMustHaveSecurityPolicySource: &configuration.SecurityPolicyControlConfig{ + Enabled: spBoolPtr(true), ExpectedProjectId: spIntPtr(9), + }, + }) + if _, ok := cfgExpect["projectMustHaveSecurityPolicySource"]; !ok { + t.Fatal("buildEngineConfig did not project a projectMustHaveSecurityPolicySource block") + } + if !fires(5, cfgExpect) { + t.Fatal("expected id 9, linked 5: expected ISSUE-601 to fire") + } + if fires(9, cfgExpect) { + t.Fatal("expected id 9, linked 9: expected no ISSUE-601") + } + + // expectedProjectId unset -> require any linkage: nothing linked fires, a linked project passes. + cfgAny := buildEngineConfig(&configuration.ControlsConfig{ + ProjectMustHaveSecurityPolicySource: &configuration.SecurityPolicyControlConfig{Enabled: spBoolPtr(true)}, + }) + if !fires(0, cfgAny) { + t.Fatal("require-any, nothing linked: expected ISSUE-601 to fire") + } + if fires(7, cfgAny) { + t.Fatal("require-any, a project linked: expected no ISSUE-601") + } + + // expectedProjectPath via the REAL projection: case-insensitive path match. + firesPath := func(linkedPath string, cfg map[string]any) bool { + p := &ir.NormalizedPipeline{Provider: ir.ProviderGitLab, SecurityPolicyProject: &ir.SecurityPolicyProjectState{Known: true, LinkedProjectID: 5, LinkedProjectPath: linkedPath}} + findings, err := engine.Evaluate(context.Background(), p, cfg) + if err != nil { + t.Fatalf("evaluate: %v", err) + } + for _, f := range findings { + if f.Code == "ISSUE-601" { + return true + } + } + return false + } + cfgPath := buildEngineConfig(&configuration.ControlsConfig{ + ProjectMustHaveSecurityPolicySource: &configuration.SecurityPolicyControlConfig{ + Enabled: spBoolPtr(true), ExpectedProjectPath: spStrPtr("Grp/Policies"), + }, + }) + if firesPath("grp/policies", cfgPath) { + t.Fatal("path match (case-insensitive): expected no ISSUE-601") + } + if !firesPath("grp/other", cfgPath) { + t.Fatal("path mismatch: expected ISSUE-601 to fire") + } +} diff --git a/control/types.go b/control/types.go index 898f7a4d..a4c9e146 100644 --- a/control/types.go +++ b/control/types.go @@ -56,6 +56,20 @@ type AnalysisResult struct { PipelineOriginData *gitlab.GitlabPipelineOriginData `json:"-"` ProtectionData *gitlab.GitlabProtectionAnalysisData `json:"-"` + // SecurityPolicyEvaluable is true when the security policy project linkage + // was read authoritatively (a successful GraphQL read). False when it could + // not be read (auth error, or the field is unavailable on the instance), so + // StatusFor reports projectMustHaveSecurityPolicySource (ISSUE-601) as + // not-evaluable rather than a false pass. + SecurityPolicyEvaluable bool `json:"-"` + + // SecurityPolicyTierCaveat is set when the security-policy control ran, the + // linkage was read, and NO policy project is linked — the tier-ambiguous + // case (Ultimate-only feature; a non-Ultimate project cannot link one, an + // Ultimate project may have left it unset). Renderers surface a conditional + // caveat next to ISSUE-601. + SecurityPolicyTierCaveat bool `json:"-"` + // GitHubStats holds per-control denominators computed from the // GitHub IR after a GitHub analysis. Used by the GitHub renderer // to produce per-control stats blocks ("Total Images: 19, diff --git a/defaultConfig/.plumber.yaml b/defaultConfig/.plumber.yaml index 9c5fcb4d..90fc4528 100644 --- a/defaultConfig/.plumber.yaml +++ b/defaultConfig/.plumber.yaml @@ -244,6 +244,24 @@ gitlab: # Minimum access level required to push (0=No one, 30=Developer, 40=Maintainer) minPushAccessLevel: 40 # =========================================== + # Project must have a security policy source + # =========================================== + # Requires the project to link a GitLab security policy project (Settings > + # Security & Compliance > Policies), which carries the org's scan-execution + # and merge-request approval policies. To require a specific policy project, + # set expectedProjectId (numeric ID) OR expectedProjectPath (full path, + # matched case-insensitively); the ID wins if both are set. Leave both unset + # to require only that SOME policy project is linked. + # + # REQUIRES GITLAB ULTIMATE: on lower tiers no policy project can be linked, + # so this fires; a conditional caveat next to the finding says so. Ships + # disabled. + projectMustHaveSecurityPolicySource: + # Set to true to enable this control + enabled: false + # expectedProjectId: 123 + # expectedProjectPath: my-group/security-policy-project + # =========================================== # Pipeline must not include hardcoded jobs # =========================================== # Detects CI/CD jobs defined directly in .gitlab-ci.yml instead of being diff --git a/docs/FINGERPRINT.md b/docs/FINGERPRINT.md index 1c32884c..55248de8 100644 --- a/docs/FINGERPRINT.md +++ b/docs/FINGERPRINT.md @@ -140,8 +140,8 @@ Finding as emitted by the rule the finding's canonical `Job` field rather than the `Data` payload bag. Most codes declare it, but nothing in the mechanism requires it: the repository- and file-level GitHub checks whose finding is not about a job leave it out -(`{file}` for ISSUE-418 / ISSUE-601, `{file, ecosystem}` for the dependabot -checks, the `{}` singleton for ISSUE-903 / ISSUE-904 / ISSUE-905), and a code +(`{file}` for ISSUE-418 / ISSUE-422, `{file, ecosystem}` for the dependabot +checks, the `{}` singleton for ISSUE-601 / ISSUE-903 / ISSUE-904 / ISSUE-905), and a code whose declaration does not name `job` does not hash on it at all. For a code that does declare it, `job` is empty when the finding is not about @@ -229,8 +229,8 @@ where the finding has no sub-finding subject: | `condition` (the `if:` expression) | ISSUE-210, ISSUE-211, ISSUE-212 | | `ecosystem` (the dependabot ecosystem) | ISSUE-901, ISSUE-902 | | `{file, job}` (one finding per job) | ISSUE-207, ISSUE-208, ISSUE-213, ISSUE-214, ISSUE-215, ISSUE-303, ISSUE-305, ISSUE-308, ISSUE-309, ISSUE-419, ISSUE-420, ISSUE-704, ISSUE-712, ISSUE-801, ISSUE-802, ISSUE-803 | -| `{file}` (one finding per workflow file) | ISSUE-418, ISSUE-601 | -| `{}` (one finding per repository) | ISSUE-903, ISSUE-904, ISSUE-905 | +| `{file}` (one finding per workflow file) | ISSUE-418, ISSUE-422 | +| `{}` (one finding per repository) | ISSUE-601, ISSUE-903, ISSUE-904, ISSUE-905 | Rewording any rule's prose no longer re-keys a registered finding. @@ -284,8 +284,8 @@ The same job cannot produce two ISSUE-803 findings, so `{file, job}` is a complete identity; two `write-all` jobs in different workflows differ on `file`. `identity.Of` reports `SubjectFromMessage == false`, and rewording the rule's message does not move the fingerprint. Coarser variants exist for -findings that are one per file (`{file}`: ISSUE-418, ISSUE-601) or one per -repository (the `{}` singleton: ISSUE-903, ISSUE-904, ISSUE-905). +findings that are one per file (`{file}`: ISSUE-418, ISSUE-422) or one per +repository (the `{}` singleton: ISSUE-601, ISSUE-903, ISSUE-904, ISSUE-905). Moving a rule from prose onto a structured payload changes its declaration and re-keys its findings once. Recipe version 2 did this for eleven finding blocks diff --git a/docs/GITHUB_ISSUES.md b/docs/GITHUB_ISSUES.md index 233c1dea..444b3175 100644 --- a/docs/GITHUB_ISSUES.md +++ b/docs/GITHUB_ISSUES.md @@ -76,7 +76,7 @@ reading the upstream docs. | Code | Name | Severity | | :--- | :--- | :--- | -| [ISSUE-601](#issue-601--anonymous-definition) | `anonymous-definition` | low | +| [ISSUE-422](#issue-422--anonymous-definition) | `anonymous-definition` | low | | [ISSUE-418](#issue-418--missing-concurrency) | `missing-concurrency` | medium | | [ISSUE-419](#issue-419--workflow-misfeature) | `workflow-misfeature` | medium | | [ISSUE-420](#issue-420--workflow-obfuscation) | `workflow-obfuscation` | high | @@ -1613,7 +1613,7 @@ jobs: --- -## ISSUE-601 — `anonymous-definition` +## ISSUE-422 — `anonymous-definition` **Severity:** `low` • **Control:** `workflowsMustHaveExplicitName` diff --git a/finding/identity/declarations.go b/finding/identity/declarations.go index a03bf30b..c7158ed8 100644 --- a/finding/identity/declarations.go +++ b/finding/identity/declarations.go @@ -157,8 +157,10 @@ var declarations = map[string][]string{ "ISSUE-501": {"file", "job", "branchName"}, // Branch protection not compliant: keyed on the branch name. "ISSUE-505": {"file", "job", "branchName"}, - // Workflow has no explicit name: one finding per workflow file, keyed on the file (benched, not yet live: declaration provisional, revisit on unbench). - "ISSUE-601": {"file"}, + // Security policy project not linked: singleton finding (one per project); the platform IdOnly was empty, so the identity is the code alone. + "ISSUE-601": {}, + // Workflow has no explicit name: one finding per workflow file, keyed on the file (benched, not yet live: declaration provisional, revisit on unbench). Renumbered from 601 when the security-policy control took 601 (#417). + "ISSUE-422": {"file"}, // Action not pinned by commit SHA: keyed on the action ref (uses); step separates a reused action. "ISSUE-701": {"file", "job", "uses", "step"}, // Action in an archived repo: keyed on the action ref (uses); step separates a reused action. diff --git a/finding/identity/identity_test.go b/finding/identity/identity_test.go index 76ccdc35..62e7717c 100644 --- a/finding/identity/identity_test.go +++ b/finding/identity/identity_test.go @@ -360,7 +360,8 @@ func TestDeclarations_EveryCodeFingerprintIsPinned(t *testing.T) { "ISSUE-421": "7573d13ae392133e", "ISSUE-501": "e36cebae06c15f85", "ISSUE-505": "4e929715c61fcba6", - "ISSUE-601": "9c1ecbe668ad9a36", + "ISSUE-601": "3a68700e66069498", + "ISSUE-422": "ade0bea017f69d56", "ISSUE-701": "87a2f87a752971bd", "ISSUE-702": "875ec32b1513e8a8", "ISSUE-703": "cf522b35974397b7", diff --git a/gitlab/dataCollectionGitlabProtection.go b/gitlab/dataCollectionGitlabProtection.go index d838a366..c3c21797 100644 --- a/gitlab/dataCollectionGitlabProtection.go +++ b/gitlab/dataCollectionGitlabProtection.go @@ -61,6 +61,15 @@ type GitlabProtectionAnalysisData struct { MRApprovalSettings *glab.ProjectApprovals `json:"mrApprovalSettings"` MRSettings *glab.Project `json:"mrSettings"` ProjectMembers []GitlabMemberInfo `json:"projectMembers"` + + // SecurityPolicyKnown is true when the security policy project linkage was + // read authoritatively (a successful GraphQL read; nil linkage then means + // "none linked"). False when the linkage could not be read (auth error, or + // the field is unavailable) so ISSUE-601 reports not-evaluable, not a pass. + SecurityPolicyKnown bool `json:"securityPolicyKnown"` + // SecurityPolicyProject is the linked GitLab security policy project, or nil + // when none is linked. Only meaningful when SecurityPolicyKnown is true. + SecurityPolicyProject *SecurityPolicyProjectLink `json:"securityPolicyProject"` } // Run fetches all GitLab protection data needed by the controls @@ -135,6 +144,19 @@ func (dc *GitlabProtectionDataCollection) Run( returnedData.ProjectMembers = members } + // Get the linked security policy project (GraphQL; GitLab Ultimate). Fetched + // only when the control is enabled — it is a separate API surface, so a + // disabled control pays no cost. A read failure is never fatal: it leaves + // SecurityPolicyKnown false, so ISSUE-601 reports not-evaluable. + if spc := conf.PlumberConfig.GetProjectMustHaveSecurityPolicySourceConfig(); spc != nil && spc.IsEnabled() { + link, known, spErr := GetSecurityPolicyProject(project.Path, token, conf.GitlabURL, conf) + if spErr != nil { + l.WithError(spErr).Warn("Failed to fetch security policy project; ISSUE-601 will report not-evaluable") + } + returnedData.SecurityPolicyKnown = known + returnedData.SecurityPolicyProject = link + } + l.WithFields(logrus.Fields{ "branchCount": len(returnedData.Branches), "branchProtectionCount": len(returnedData.BranchProtections), diff --git a/gitlab/gitlab_ir.go b/gitlab/gitlab_ir.go index 4c5d8f7f..aaa5c788 100644 --- a/gitlab/gitlab_ir.go +++ b/gitlab/gitlab_ir.go @@ -47,6 +47,7 @@ func ToNormalizedPipeline( pipeline.Includes = buildIncludes(origin, ciConfigPath) pipeline.Jobs = buildJobs(origin, imagesByJob, ciConfigPath, pipeline.Includes) pipeline.Branches = buildBranches(protection) + pipeline.SecurityPolicyProject = buildSecurityPolicyProject(protection) if origin != nil && origin.MergedConf != nil { if globals := extractGitLabVariables(origin.MergedConf.GlobalVariables); len(globals) > 0 { pipeline.GlobalVariables = globals @@ -61,6 +62,27 @@ func ToNormalizedPipeline( return pipeline } +// buildSecurityPolicyProject projects the collected security policy project +// linkage onto the IR. Returns nil when the linkage was not collected (the +// control disabled) so the rule sees no field and abstains. When collected, the +// Known flag carries whether the read was authoritative; a Known projection with +// LinkedProjectID == 0 means "no policy project linked". +func buildSecurityPolicyProject(protection *GitlabProtectionAnalysisData) *ir.SecurityPolicyProjectState { + if protection == nil { + return nil + } + // Not collected at all (control disabled): no Known flag, no linkage. + if !protection.SecurityPolicyKnown && protection.SecurityPolicyProject == nil { + return nil + } + state := &ir.SecurityPolicyProjectState{Known: protection.SecurityPolicyKnown} + if protection.SecurityPolicyProject != nil { + state.LinkedProjectID = protection.SecurityPolicyProject.ID + state.LinkedProjectPath = protection.SecurityPolicyProject.FullPath + } + return state +} + // buildBranches flattens the GitLab protection API response into // ir.Branch entries. Each repository branch is matched against the // declared protection patterns; when a pattern matches, its settings diff --git a/gitlab/gitlab_ir_test.go b/gitlab/gitlab_ir_test.go index e018aa51..1c9c1e16 100644 --- a/gitlab/gitlab_ir_test.go +++ b/gitlab/gitlab_ir_test.go @@ -6,6 +6,57 @@ import ( "github.com/getplumber/plumber/internal/ir" ) +// TestBuildSecurityPolicyProject pins the collector-data -> IR projection that +// decides abstain vs fire for ISSUE-601. The subtle case is a successful read +// with nothing linked (Known=true, no project): the projection must return a +// non-nil state with LinkedProjectID 0 so the rule's require-any mode fires, +// rather than nil (which would abstain and silently miss an unlinked project). +func TestBuildSecurityPolicyProject(t *testing.T) { + cases := []struct { + name string + in *GitlabProtectionAnalysisData + want *ir.SecurityPolicyProjectState + }{ + { + name: "nil protection -> nil (not collected)", + in: nil, + want: nil, + }, + { + name: "not collected (Known=false, no project) -> nil (abstain)", + in: &GitlabProtectionAnalysisData{SecurityPolicyKnown: false, SecurityPolicyProject: nil}, + want: nil, + }, + { + name: "read OK, none linked -> non-nil Known with id 0 (must fire)", + in: &GitlabProtectionAnalysisData{SecurityPolicyKnown: true, SecurityPolicyProject: nil}, + want: &ir.SecurityPolicyProjectState{Known: true, LinkedProjectID: 0, LinkedProjectPath: ""}, + }, + { + name: "read OK, one linked -> non-nil Known with id/path", + in: &GitlabProtectionAnalysisData{SecurityPolicyKnown: true, SecurityPolicyProject: &SecurityPolicyProjectLink{ID: 42, FullPath: "grp/pol"}}, + want: &ir.SecurityPolicyProjectState{Known: true, LinkedProjectID: 42, LinkedProjectPath: "grp/pol"}, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := buildSecurityPolicyProject(tc.in) + if tc.want == nil { + if got != nil { + t.Fatalf("expected nil projection, got %+v", got) + } + return + } + if got == nil { + t.Fatalf("expected non-nil projection %+v, got nil", tc.want) + } + if got.Known != tc.want.Known || got.LinkedProjectID != tc.want.LinkedProjectID || got.LinkedProjectPath != tc.want.LinkedProjectPath { + t.Fatalf("projection mismatch: got %+v, want %+v", got, tc.want) + } + }) + } +} + func TestToNormalizedPipeline_Empty(t *testing.T) { pipeline := ToNormalizedPipeline("group/project", "main", "", nil, nil, nil) if pipeline.Provider != ir.ProviderGitLab { diff --git a/gitlab/security_policy.go b/gitlab/security_policy.go new file mode 100644 index 00000000..a15d0350 --- /dev/null +++ b/gitlab/security_policy.go @@ -0,0 +1,105 @@ +package gitlab + +import ( + "context" + "strconv" + "strings" + + "github.com/getplumber/plumber/configuration" + "github.com/machinebox/graphql" + "github.com/sirupsen/logrus" +) + +// SecurityPolicyProjectLink is the GitLab security policy project linked to the +// analysed project. GitLab Ultimate: a project points at a single security +// policy project that carries the org's scan-execution and merge-request +// approval policies. +type SecurityPolicyProjectLink struct { + // ID is the numeric project ID of the linked security policy project. + ID int + // FullPath is its namespace/path. + FullPath string +} + +// GetSecurityPolicyProject fetches the project's linked security policy project +// via GraphQL. It returns: +// - (link, true, nil) on a successful read where a policy project is linked; +// - (nil, true, nil) on a successful read where NONE is linked (the +// GitLab-Free/Ultimate-but-unlinked case — the field answers null); +// - (nil, false, err) when the linkage could not be read authoritatively (an +// auth error, or the field is unavailable on the instance). The bool is the +// "known" flag: a false known maps to not-evaluable, never a false pass. +// +// Security policies require GitLab Ultimate. On a non-Ultimate project the field +// answers null (no linkage), which is indistinguishable from an Ultimate project +// that simply has not linked one — the caller surfaces a conditional tier caveat +// rather than asserting the tier. +func GetSecurityPolicyProject(fullPath, token, instanceUrl string, conf *configuration.Configuration) (*SecurityPolicyProjectLink, bool, error) { + l := logrus.WithFields(logrus.Fields{ + "platform": "gitlab", + "action": "GetSecurityPolicyProject", + "projectFullPath": fullPath, + "instanceUrl": instanceUrl, + }) + + request := ` + query getSecurityPolicyProject($fullPath: ID!) { + project(fullPath: $fullPath) { + securityPolicyProject { + id + fullPath + } + } + } + ` + + type policyProject struct { + ID string `json:"id"` + FullPath string `json:"fullPath"` + } + type response struct { + Project *struct { + SecurityPolicyProject *policyProject `json:"securityPolicyProject"` + } `json:"project"` + } + + client := GetGraphQLClient(instanceUrl, conf) + req := graphql.NewRequest(request) + req.Var("fullPath", fullPath) + req.Header.Add("Authorization", "Bearer "+token) + + var respData response + if err := client.Run(context.Background(), req, &respData); err != nil { + // The field is absent from this instance's schema (old or unlicensed + // self-managed): treat as not-evaluable rather than a failure, matching + // the platform's "continue without security policy data" handling. + if strings.Contains(err.Error(), "securityPolicyProject") && strings.Contains(err.Error(), "doesn't exist") { + l.WithError(err).Warning("securityPolicyProject field unavailable on this GitLab instance; reporting not-evaluable") + return nil, false, nil + } + l.WithError(err).Error("Failed to read the security policy project through the GitLab GraphQL API") + return nil, false, err + } + + if respData.Project == nil || respData.Project.SecurityPolicyProject == nil { + return nil, true, nil // read succeeded; nothing linked + } + p := respData.Project.SecurityPolicyProject + return &SecurityPolicyProjectLink{ID: parseGitlabGID(p.ID), FullPath: p.FullPath}, true, nil +} + +// parseGitlabGID extracts the trailing numeric id from a GitLab GraphQL global +// id such as "gid://gitlab/Project/12345". Returns 0 when the tail is not a +// number (an unexpected id shape), so a malformed id never matches a configured +// expectedProjectId by accident. +func parseGitlabGID(gid string) int { + idx := strings.LastIndex(gid, "/") + if idx < 0 || idx+1 >= len(gid) { + return 0 + } + n, err := strconv.Atoi(gid[idx+1:]) + if err != nil { + return 0 + } + return n +} diff --git a/gitlab/security_policy_test.go b/gitlab/security_policy_test.go new file mode 100644 index 00000000..b9ded0f6 --- /dev/null +++ b/gitlab/security_policy_test.go @@ -0,0 +1,101 @@ +package gitlab + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/getplumber/plumber/configuration" +) + +// TestGetSecurityPolicyProject pins the four read outcomes the ISSUE-601 +// not-evaluable design depends on: a linked project, no linkage, the +// field-unavailable case, and an auth error. +func TestGetSecurityPolicyProject(t *testing.T) { + conf := &configuration.Configuration{HTTPClientTimeout: 30 * time.Second} + + t.Run("linked -> parsed id/path, known", func(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/api/graphql", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{"data": map[string]any{ + "project": map[string]any{"securityPolicyProject": map[string]any{ + "id": "gid://gitlab/Project/4242", "fullPath": "grp/security-policies", + }}, + }}) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + link, known, err := GetSecurityPolicyProject("grp/app", "tok", srv.URL, conf) + if err != nil || !known { + t.Fatalf("expected a known success, got known=%v err=%v", known, err) + } + if link == nil || link.ID != 4242 || link.FullPath != "grp/security-policies" { + t.Fatalf("unexpected link: %+v", link) + } + }) + + t.Run("none linked -> nil link, known", func(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/api/graphql", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{"data": map[string]any{ + "project": map[string]any{"securityPolicyProject": nil}, + }}) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + link, known, err := GetSecurityPolicyProject("grp/app", "tok", srv.URL, conf) + if err != nil || !known || link != nil { + t.Fatalf("none linked: expected (nil, true, nil), got (%+v, %v, %v)", link, known, err) + } + }) + + t.Run("field unavailable -> not-evaluable, no error", func(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/api/graphql", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{"errors": []map[string]any{ + {"message": "Field 'securityPolicyProject' doesn't exist on type 'Project'"}, + }}) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + link, known, err := GetSecurityPolicyProject("grp/app", "tok", srv.URL, conf) + if err != nil || known || link != nil { + t.Fatalf("field unavailable: expected (nil, false, nil), got (%+v, %v, %v)", link, known, err) + } + }) + + t.Run("auth error -> not-evaluable with error", func(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/api/graphql", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + link, known, err := GetSecurityPolicyProject("grp/app", "tok", srv.URL, conf) + if err == nil || known || link != nil { + t.Fatalf("auth error: expected (nil, false, err), got (%+v, %v, %v)", link, known, err) + } + }) +} + +func TestParseGitlabGID(t *testing.T) { + cases := map[string]int{ + "gid://gitlab/Project/12345": 12345, + "gid://gitlab/Project/1": 1, + "": 0, + "gid://gitlab/Project/": 0, + "not-a-gid": 0, + "gid://gitlab/Project/abc": 0, + } + for in, want := range cases { + if got := parseGitlabGID(in); got != want { + t.Errorf("parseGitlabGID(%q) = %d, want %d", in, got, want) + } + } +} diff --git a/internal/ir/pipeline.go b/internal/ir/pipeline.go index 46fb91a7..d8d70827 100644 --- a/internal/ir/pipeline.go +++ b/internal/ir/pipeline.go @@ -66,6 +66,15 @@ type NormalizedPipeline struct { // (root, .github/, or docs/). Empty when the file is absent. SecurityPolicyPath string `json:"securityPolicyPath,omitempty"` + // SecurityPolicyProject is the GitLab security policy project linked to this + // project (Settings > Security & Compliance > Policies), projected from the + // protection collection. nil when the linkage was not collected (the control + // is disabled) or could not be read authoritatively, so the + // projectMustHaveSecurityPolicySource control (ISSUE-601) abstains and + // reports not-evaluable. Distinct from SecurityPolicyPath above, which is the + // repository's SECURITY.md file (a GitHub-oriented, unrelated control). + SecurityPolicyProject *SecurityPolicyProjectState `json:"securityPolicyProject,omitempty"` + // Dockerfiles lists every Dockerfile the collector scanned at the // repo root and under common build directories, with each FROM // base-image extracted so policies can check pinning state. @@ -83,6 +92,21 @@ type NormalizedPipeline struct { Raw map[string]any `json:"raw,omitempty"` } +// SecurityPolicyProjectState is the GitLab security policy project linkage +// projected onto the IR. Known is true when the linkage was read +// authoritatively; a Known projection with LinkedProjectID == 0 means no policy +// project is linked. When Known is false the rule abstains (not-evaluable). +type SecurityPolicyProjectState struct { + // Known is true when the linkage was read authoritatively. + Known bool `json:"known"` + // LinkedProjectID is the numeric ID of the linked security policy project, + // or 0 when none is linked. + LinkedProjectID int `json:"linkedProjectId"` + // LinkedProjectPath is the full path of the linked security policy project, + // or "" when none is linked. + LinkedProjectPath string `json:"linkedProjectPath"` +} + // Dockerfile captures the result of parsing a single Dockerfile's // FROM directives for supply-chain auditing. type Dockerfile struct { diff --git a/policies/anonymous_definition.rego b/policies/anonymous_definition.rego index ee6404de..ea050a7b 100644 --- a/policies/anonymous_definition.rego +++ b/policies/anonymous_definition.rego @@ -16,7 +16,7 @@ deny contains finding if { input.pipeline.provider == "github" some file in _anonymous_workflow_files finding := { - "code": "ISSUE-601", + "code": "ISSUE-422", "severity": "low", "message": sprintf("workflow file %q has no top-level `name:` — GitHub falls back to the file path", [file]), "file": file, diff --git a/policies/rules_test.go b/policies/rules_test.go index 4dfdcd8b..060411d1 100644 --- a/policies/rules_test.go +++ b/policies/rules_test.go @@ -3333,9 +3333,79 @@ func TestIssue705_CachePoisoning_Configurable(t *testing.T) { }) } -// TestIssue601_AnonymousDefinition flags workflow files without a -// top-level `name:`. One finding per file (not per job). -func TestIssue601_AnonymousDefinition(t *testing.T) { +// TestIssue601_SecurityPolicyProject flags a GitLab project that does not link +// the expected security policy project. Singleton. Abstains when the linkage +// could not be read (known=false) or was not collected. +func TestIssue601_SecurityPolicyProject(t *testing.T) { + engine := opaengine.New() + if err := engine.LoadFromFSFiltered(policies.FS, nil); err != nil { + t.Fatalf("load embedded policies: %v", err) + } + count601 := func(p *ir.NormalizedPipeline, cfg map[string]any) int { + findings, err := engine.Evaluate(context.Background(), p, cfg) + if err != nil { + t.Fatalf("evaluate: %v", err) + } + n := 0 + for _, f := range findings { + if f.Code == "ISSUE-601" { + n++ + } + } + return n + } + gl := func(sp *ir.SecurityPolicyProjectState) *ir.NormalizedPipeline { + return &ir.NormalizedPipeline{Provider: ir.ProviderGitLab, SecurityPolicyProject: sp} + } + anyLinkage := map[string]any{"projectMustHaveSecurityPolicySource": map[string]any{}} + expect9 := map[string]any{"projectMustHaveSecurityPolicySource": map[string]any{"expectedProjectId": 9}} + + // Require-any: nothing linked -> fires; something linked -> passes. + if got := count601(gl(&ir.SecurityPolicyProjectState{Known: true, LinkedProjectID: 0}), anyLinkage); got != 1 { + t.Fatalf("require-any, nothing linked: expected 1 ISSUE-601, got %d", got) + } + if got := count601(gl(&ir.SecurityPolicyProjectState{Known: true, LinkedProjectID: 5, LinkedProjectPath: "grp/pol"}), anyLinkage); got != 0 { + t.Fatalf("require-any, a project linked: expected 0 ISSUE-601, got %d", got) + } + + // Expected id: wrong linked -> fires; matching -> passes. + if got := count601(gl(&ir.SecurityPolicyProjectState{Known: true, LinkedProjectID: 5}), expect9); got != 1 { + t.Fatalf("expected id 9, linked 5: expected 1 ISSUE-601, got %d", got) + } + if got := count601(gl(&ir.SecurityPolicyProjectState{Known: true, LinkedProjectID: 9}), expect9); got != 0 { + t.Fatalf("expected id 9, linked 9: expected 0 ISSUE-601, got %d", got) + } + + // Path mode: expectedProjectPath, compared case-insensitively. + expectPath := map[string]any{"projectMustHaveSecurityPolicySource": map[string]any{"expectedProjectPath": "Grp/Policies"}} + if got := count601(gl(&ir.SecurityPolicyProjectState{Known: true, LinkedProjectID: 5, LinkedProjectPath: "grp/policies"}), expectPath); got != 0 { + t.Fatalf("path mode, case-insensitive match: expected 0 ISSUE-601, got %d", got) + } + if got := count601(gl(&ir.SecurityPolicyProjectState{Known: true, LinkedProjectID: 5, LinkedProjectPath: "grp/other"}), expectPath); got != 1 { + t.Fatalf("path mode, mismatch: expected 1 ISSUE-601, got %d", got) + } + + // Precedence: when both id and path are set, the id is authoritative — a + // matching id passes even if the path would mismatch. + both := map[string]any{"projectMustHaveSecurityPolicySource": map[string]any{"expectedProjectId": 9, "expectedProjectPath": "grp/other"}} + if got := count601(gl(&ir.SecurityPolicyProjectState{Known: true, LinkedProjectID: 9, LinkedProjectPath: "grp/policies"}), both); got != 0 { + t.Fatalf("id precedence: matching id must pass despite path mismatch, got %d ISSUE-601", got) + } + + // Abstain: linkage not read authoritatively (known=false) -> no finding even + // with an expectation, and no projection at all -> no finding. + if got := count601(gl(&ir.SecurityPolicyProjectState{Known: false}), expect9); got != 0 { + t.Fatalf("known=false must abstain (not-evaluable), got %d ISSUE-601", got) + } + if got := count601(gl(nil), expect9); got != 0 { + t.Fatalf("no security-policy projection must abstain, got %d ISSUE-601", got) + } +} + +// TestIssue422_AnonymousDefinition flags workflow files without a +// top-level `name:`. One finding per file (not per job). Renumbered from +// ISSUE-601 when the security-policy control took 601 (#417). +func TestIssue422_AnonymousDefinition(t *testing.T) { cases := []struct { fixture string wantCount int @@ -3355,7 +3425,7 @@ func TestIssue601_AnonymousDefinition(t *testing.T) { if err := os.MkdirAll(wfDir, 0o755); err != nil { t.Fatal(err) } - src := filepath.Join("testdata", "ISSUE-601", "github", tc.fixture) + src := filepath.Join("testdata", "ISSUE-422", "github", tc.fixture) data, err := os.ReadFile(src) if err != nil { t.Fatalf("read fixture: %v", err) @@ -3373,7 +3443,7 @@ func TestIssue601_AnonymousDefinition(t *testing.T) { } hits := 0 for _, f := range findings { - if f.Code == "ISSUE-601" { + if f.Code == "ISSUE-422" { hits++ } } @@ -3384,9 +3454,9 @@ func TestIssue601_AnonymousDefinition(t *testing.T) { } } -// TestIssue602_MissingConcurrency flags workflow files with no +// TestIssue422_MissingConcurrency flags workflow files with no // concurrency block at either workflow or job level. -func TestIssue602_MissingConcurrency(t *testing.T) { +func TestIssue422_MissingConcurrency(t *testing.T) { cases := []struct { fixture string wantCount int diff --git a/policies/security_policy_project.rego b/policies/security_policy_project.rego new file mode 100644 index 00000000..3353db0f --- /dev/null +++ b/policies/security_policy_project.rego @@ -0,0 +1,91 @@ +# security-policy-project — flag a GitLab project that does not directly link +# the expected security policy project (Settings > Security & Compliance > +# Policies). A linked security policy project carries the organization's +# scan-execution and merge-request approval policies. This checks the project's +# own link only, matching the legacy platform: a policy source inherited from a +# parent group is not detected (GitLab's project-scoped securityPolicyProject +# field is null for an inherited source), and the linked project's policy +# contents are not inspected, so "linked" means a source is attached, not that a +# policy is necessarily enforced. GitLab-only singleton finding (one per +# project); the legacy platform's identity was empty, so the identity here is +# the code alone. +# +# Config projectMustHaveSecurityPolicySource, matched with this precedence: +# - expectedProjectId set => the linked project's numeric id must equal it +# exactly (authoritative; the front end always sends the id); +# - else expectedProjectPath set => the linked project's full path must equal +# it, compared case-insensitively (a human-friendly alternative); +# - else (neither set) => any linked policy project passes, and the +# control fails only when none is linked. +# +# Reads input.pipeline.securityPolicyProject, projected from the protection +# collection (gitlab/gitlab_ir.go::buildSecurityPolicyProject). The projection +# is absent when the control did not collect it, and carries known=false when +# the linkage could not be read (a 401/403, or the field is unavailable on the +# instance); the rule abstains in both cases, so the control reports +# not-evaluable, not a pass. +# +# Security policies require GitLab Ultimate. On a non-Ultimate project the +# linkage reads as none, which is indistinguishable from an Ultimate project +# that has not linked one — the Go layer surfaces a conditional Ultimate tier +# caveat next to this finding rather than the rule asserting the tier. +package security_policy_project + +import rego.v1 + +deny contains finding if { + input.pipeline.provider == "gitlab" + sp := input.pipeline.securityPolicyProject + sp.known == true + cfg := object.get(input.config, "projectMustHaveSecurityPolicySource", {}) + finding := { + "code": "ISSUE-601", + "severity": "critical", + "message": _violation(sp, cfg), + "linkedProjectId": sp.linkedProjectId, + "linkedProjectPath": sp.linkedProjectPath, + } +} + +# expectedProjectId 0 (or absent) means "no id configured"; GitLab project ids +# start at 1, so 0 is a safe sentinel. expectedProjectPath "" means "no path +# configured". The three modes below are mutually exclusive by their guards, so +# exactly one _violation body can match: id wins, then path, then any-linkage. +_expected_id(cfg) := object.get(cfg, "expectedProjectId", 0) + +_expected_path(cfg) := object.get(cfg, "expectedProjectPath", "") + +# Normalise a path for comparison: trim surrounding slashes and lowercase, since +# GitLab namespaces are case-insensitive. +_norm(p) := trim(lower(p), "/") + +# ID mode (id set, authoritative): the linked id must equal it. +_violation(sp, cfg) := _mismatch_msg(sp, sprintf("id %d", [_expected_id(cfg)])) if { + _expected_id(cfg) != 0 + sp.linkedProjectId != _expected_id(cfg) +} + +# Path mode (no id, path set): the linked path must equal it, normalised. +_violation(sp, cfg) := _mismatch_msg(sp, sprintf("path %q", [_expected_path(cfg)])) if { + _expected_id(cfg) == 0 + _expected_path(cfg) != "" + _norm(sp.linkedProjectPath) != _norm(_expected_path(cfg)) +} + +# Any-linkage mode (neither set): fail only when nothing is linked. +_violation(sp, cfg) := "no GitLab security policy project is linked to this project" if { + _expected_id(cfg) == 0 + _expected_path(cfg) == "" + sp.linkedProjectId == 0 +} + +_mismatch_msg(sp, want) := sprintf("no GitLab security policy project is linked (expected %s)", [want]) if { + sp.linkedProjectId == 0 +} + +_mismatch_msg(sp, want) := sprintf( + "the linked GitLab security policy project (id %d, path %q) is not the expected project (%s)", + [sp.linkedProjectId, sp.linkedProjectPath, want], +) if { + sp.linkedProjectId != 0 +} diff --git a/policies/testdata/ISSUE-601/github/clean_named.yml b/policies/testdata/ISSUE-422/github/clean_named.yml similarity index 100% rename from policies/testdata/ISSUE-601/github/clean_named.yml rename to policies/testdata/ISSUE-422/github/clean_named.yml diff --git a/policies/testdata/ISSUE-601/github/violation_unnamed.yml b/policies/testdata/ISSUE-422/github/violation_unnamed.yml similarity index 100% rename from policies/testdata/ISSUE-601/github/violation_unnamed.yml rename to policies/testdata/ISSUE-422/github/violation_unnamed.yml