diff --git a/.plumber.yaml b/.plumber.yaml index 54d4d379..0c8cf1ea 100644 --- a/.plumber.yaml +++ b/.plumber.yaml @@ -147,6 +147,20 @@ gitlab: - gcr.io/go-containerregistry/* # google/go-containerregistry (crane, gcrane) - gcr.io/google.com/cloudsdktool/* # Google Cloud SDK (gcloud) official # =========================================== + # CI/CD variables must be protected + # =========================================== + # Flags project CI/CD settings variables that are not marked + # protected, so they are exposed to pipelines on unprotected + # branches. GitLab-only. + cicdVariablesMustBeProtected: + enabled: true + # =========================================== + # CI/CD variables must be masked + # =========================================== + # Flags project CI/CD settings variables that are not masked, so + # their values print in job logs. GitLab-only. + cicdVariablesMustBeMasked: + enabled: true # Branch must be protected # =========================================== # Checks that repository branches have proper protection settings. diff --git a/README.md b/README.md index d6f95cce..de51abb4 100644 --- a/README.md +++ b/README.md @@ -313,6 +313,7 @@ Plumber ships controls for: - container image pinning and authorized sources - branch protection +- GitLab CI/CD settings variables that must be protected and masked - unverified script execution (`curl | bash`, `base64 -d | bash`, etc.) - Docker-in-Docker - weakened security jobs diff --git a/cmd/init.go b/cmd/init.go index 3743aacf..c86e56d3 100644 --- a/cmd/init.go +++ b/cmd/init.go @@ -28,7 +28,7 @@ const ( catImages = "Container image security (tags, trusted registries)" catComposition = "Pipeline composition (includes, scripts, security jobs, DinD)" catAccess = "Access control (branch protection)" - catVariables = "Variable security (debug trace, unsafe expansion)" + catVariables = "Variable security (settings variables, debug trace, unsafe expansion)" // GitLab-applicable composition checks (existing). compHardcoded = "Disallow hardcoded jobs (use includes/components)" @@ -146,6 +146,8 @@ type initWizardState struct { DebugTraceEnabled bool UnsafeExpansionEnabled bool + VarsProtectedEnabled bool + VarsMaskedEnabled bool RequireComponents bool RequiredComponentsExpr string @@ -504,6 +506,12 @@ func (st *initWizardState) askAccessQuestions() error { func (st *initWizardState) askVariableQuestions() error { printInitSection("Variable security") + if err := survey.AskOne(&survey.Confirm{Message: "Flag CI/CD settings variables (Settings > CI/CD > Variables) that are not protected?", Help: "Unprotected variables are exposed to pipelines on unprotected branches. Requires a GitLab token that can read variables. Ships off by default.", Default: defaultCicdVariablesProtectedEnabled()}, &st.VarsProtectedEnabled); err != nil { + return err + } + if err := survey.AskOne(&survey.Confirm{Message: "Flag CI/CD settings variables that are not masked?", Help: "Unmasked variables print verbatim in job logs. Ships off by default.", Default: defaultCicdVariablesMaskedEnabled()}, &st.VarsMaskedEnabled); err != nil { + return err + } if err := survey.AskOne(&survey.Confirm{Message: "Flag pipelines that enable CI_DEBUG_TRACE or CI_DEBUG_SERVICES?", Default: true}, &st.DebugTraceEnabled); err != nil { return err } @@ -773,6 +781,23 @@ var embeddedDefault = sync.OnceValue(func() *configuration.PlumberConfig { func defaultGitLabControls() configuration.ControlsConfig { return embeddedDefault().GitLab.Controls } func defaultGitHubControls() configuration.ControlsConfig { return embeddedDefault().GitHub.Controls } +// defaultCicdVariablesProtectedEnabled / ...Masked source the wizard's +// Confirm defaults from the shipped default (both ship disabled), so the +// wizard prompt and the zero-config baseline cannot drift. +func defaultCicdVariablesProtectedEnabled() bool { + if c := defaultGitLabControls().CicdVariablesMustBeProtected; c != nil { + return c.IsEnabled() + } + return false +} + +func defaultCicdVariablesMaskedEnabled() bool { + if c := defaultGitLabControls().CicdVariablesMustBeMasked; c != nil { + return c.IsEnabled() + } + return false +} + // defaultForbiddenTags is the CSV prompt default for forbidden image tags, // sourced from the GitLab containerImageMustNotUseForbiddenTags default. func defaultForbiddenTags() string { @@ -1045,6 +1070,12 @@ func (st *initWizardState) applyVariableControls(gl *configuration.ProviderConfi AllowedPatterns: parseLinesInit(st.AllowedPatternsMultiline), } } + if st.VarsProtectedEnabled { + gl.Controls.CicdVariablesMustBeProtected = &configuration.EnabledOnlyControlConfig{Enabled: boolPtrInit(true)} + } + if st.VarsMaskedEnabled { + gl.Controls.CicdVariablesMustBeMasked = &configuration.EnabledOnlyControlConfig{Enabled: boolPtrInit(true)} + } } func (st *initWizardState) toPlumberConfig() *configuration.PlumberConfig { diff --git a/cmd/init_variables_test.go b/cmd/init_variables_test.go new file mode 100644 index 00000000..810818e3 --- /dev/null +++ b/cmd/init_variables_test.go @@ -0,0 +1,42 @@ +package cmd + +import ( + "testing" + + "github.com/getplumber/plumber/configuration" +) + +// applyVariableControls maps the two wizard booleans onto config: +// VarsProtectedEnabled -> CicdVariablesMustBeProtected and +// VarsMaskedEnabled -> CicdVariablesMustBeMasked. A field mix-up would silently +// produce a wrong .plumber.yaml from `plumber config init`, and no drift guard +// catches it (both controls ship disabled). This pins the mapping. +func TestApplyVariableControls_Mapping(t *testing.T) { + t.Run("protected only sets only the protected control", func(t *testing.T) { + gl := &configuration.ProviderConfig{} + (&initWizardState{VarsProtectedEnabled: true}).applyVariableControls(gl) + if gl.Controls.CicdVariablesMustBeProtected == nil || !gl.Controls.CicdVariablesMustBeProtected.IsEnabled() { + t.Fatal("protected control should be enabled") + } + if gl.Controls.CicdVariablesMustBeMasked != nil { + t.Fatal("masked control must stay unset when only protected was chosen (field mix-up)") + } + }) + t.Run("masked only sets only the masked control", func(t *testing.T) { + gl := &configuration.ProviderConfig{} + (&initWizardState{VarsMaskedEnabled: true}).applyVariableControls(gl) + if gl.Controls.CicdVariablesMustBeMasked == nil || !gl.Controls.CicdVariablesMustBeMasked.IsEnabled() { + t.Fatal("masked control should be enabled") + } + if gl.Controls.CicdVariablesMustBeProtected != nil { + t.Fatal("protected control must stay unset when only masked was chosen (field mix-up)") + } + }) + t.Run("neither sets nothing", func(t *testing.T) { + gl := &configuration.ProviderConfig{} + (&initWizardState{}).applyVariableControls(gl) + if gl.Controls.CicdVariablesMustBeProtected != nil || gl.Controls.CicdVariablesMustBeMasked != nil { + t.Fatal("no variable controls should be set when neither was chosen") + } + }) +} diff --git a/cmd/legacy_json.go b/cmd/legacy_json.go index 0489987f..182968dd 100644 --- a/cmd/legacy_json.go +++ b/cmd/legacy_json.go @@ -144,10 +144,59 @@ func buildLegacyResult(e control.ControlEntry, result *control.AnalysisResult, p return "jobVariablesOverrideResult", buildJobVariablesOverrideBlock(common, result, findings) case "pipelineMustNotUseDockerInDocker": return "dockerInDockerResult", buildDockerInDockerBlock(common, result, findings) + case "cicdVariablesMustBeProtected": + return "cicdVariablesProtectedResult", buildCicdVariablesProtectedBlock(common, result, findings) + case "cicdVariablesMustBeMasked": + return "cicdVariablesMaskedResult", buildCicdVariablesMaskedBlock(common, result, findings) } return "", nil } +// buildCicdVariablesProtectedBlock and buildCicdVariablesMaskedBlock emit the +// legacy JSON blocks for the settings-variable controls. The findings are +// settings-level (no file/job), so each issue carries the variable identity +// (variableName / variableType / environment) that projectFindings preserves; +// the value is never present, per the #370 variable-sensitivity tiers. +func buildCicdVariablesProtectedBlock(c legacyCommon, result *control.AnalysisResult, findings []opaengine.Finding) map[string]any { + return map[string]any{ + "issues": projectFindings(findings, "job"), + "metrics": map[string]any{ + "totalVariablesChecked": variablesCheckedCount(result), + "unprotectedFound": len(findings), + }, + "version": "0.1.0", + "ciValid": c.CiValid, + "ciMissing": c.CiMissing, + "skipped": c.Skipped, + } +} + +func buildCicdVariablesMaskedBlock(c legacyCommon, result *control.AnalysisResult, findings []opaengine.Finding) map[string]any { + return map[string]any{ + "issues": projectFindings(findings, "job"), + "metrics": map[string]any{ + "totalVariablesChecked": variablesCheckedCount(result), + "unmaskedFound": len(findings), + }, + "version": "0.1.0", + "ciValid": c.CiValid, + "ciMissing": c.CiMissing, + "skipped": c.Skipped, + } +} + +// variablesCheckedCount is the denominator for the settings-variable JSON +// blocks: how many variables were read (0 when the listing was unreadable or +// not collected — the block's status:error then carries the not-evaluable +// signal). Mirrors the totalVariablesChecked metric on the comparable +// pipelineMustNotOverrideJobVariables block. +func variablesCheckedCount(result *control.AnalysisResult) int { + if result != nil && result.VariablesData != nil { + return len(result.VariablesData.Variables) + } + return 0 +} + // legacyCommon carries the bookkeeping fields shared by every // `*Result` block: ciValid, ciMissing, skipped. type legacyCommon struct { diff --git a/cmd/legacy_json_variables_test.go b/cmd/legacy_json_variables_test.go new file mode 100644 index 00000000..21fb1ef4 --- /dev/null +++ b/cmd/legacy_json_variables_test.go @@ -0,0 +1,68 @@ +package cmd + +import ( + "testing" + + "github.com/getplumber/plumber/control" + "github.com/getplumber/plumber/gitlab" + opaengine "github.com/getplumber/plumber/internal/engine/opa" +) + +// TestCicdVariablesJSONBlocks locks the machine-readable legacy-JSON contract +// for the two settings-variable controls: the top-level result key, the metric +// key, and which builder each control routes to are all hand-wired and mutually +// confusable (protected vs masked). A swapped block or a copy-pasted metric key +// would silently emit the wrong shape to JSON consumers with no other guard. +func TestCicdVariablesJSONBlocks(t *testing.T) { + // Five variables read: the JSON denominator totalVariablesChecked must + // report that regardless of how many were flagged. + result := &control.AnalysisResult{ + CiValid: true, + VariablesData: &gitlab.GitlabVariablesAnalysisData{Variables: make([]gitlab.CICDVariable, 5), Known: true}, + } + + // Protected -> cicdVariablesProtectedResult, metric unprotectedFound. + protEntry := control.ControlEntry{ControlName: "cicdVariablesMustBeProtected"} + protFindings := []opaengine.Finding{ + {Code: "ISSUE-201", Data: map[string]any{"variableName": "AWS_KEY", "variableType": "env_var", "environment": "*"}}, + {Code: "ISSUE-201", Data: map[string]any{"variableName": "DEPLOY", "variableType": "file", "environment": "production"}}, + } + name, block := buildLegacyResult(protEntry, result, nil, protFindings) + if name != "cicdVariablesProtectedResult" { + t.Fatalf("protected block name = %q, want cicdVariablesProtectedResult (routing/copy-paste)", name) + } + m := block.(map[string]any) + if metrics, ok := m["metrics"].(map[string]any); !ok || metrics["unprotectedFound"] != 2 || metrics["totalVariablesChecked"] != 5 { + t.Errorf("protected metrics = %v, want unprotectedFound=2 totalVariablesChecked=5", m["metrics"]) + } + issues, ok := m["issues"].([]map[string]any) + if !ok || len(issues) != 2 { + t.Fatalf("protected issues = %v, want 2", m["issues"]) + } + // Settings-level findings carry the variable identity and no job key. + if issues[0]["variableName"] == nil || issues[0]["variableType"] == nil || issues[0]["environment"] == nil { + t.Errorf("issue must preserve variableName/variableType/environment: %v", issues[0]) + } + if _, hasJob := issues[0]["job"]; hasJob { + t.Errorf("settings-level finding must carry no job key: %v", issues[0]) + } + + // Masked -> cicdVariablesMaskedResult, metric unmaskedFound, and must NOT + // leak the protected metric key. + maskEntry := control.ControlEntry{ControlName: "cicdVariablesMustBeMasked"} + maskFindings := []opaengine.Finding{ + {Code: "ISSUE-202", Data: map[string]any{"variableName": "PLAIN", "variableType": "env_var", "environment": "*"}}, + } + name, block = buildLegacyResult(maskEntry, result, nil, maskFindings) + if name != "cicdVariablesMaskedResult" { + t.Fatalf("masked block name = %q, want cicdVariablesMaskedResult (routing/copy-paste)", name) + } + m = block.(map[string]any) + metrics, ok := m["metrics"].(map[string]any) + if !ok || metrics["unmaskedFound"] != 1 || metrics["totalVariablesChecked"] != 5 { + t.Errorf("masked metrics = %v, want unmaskedFound=1 totalVariablesChecked=5", m["metrics"]) + } + if _, wrong := metrics["unprotectedFound"]; wrong { + t.Errorf("masked block leaked the protected metric key unprotectedFound: %v", metrics) + } +} diff --git a/cmd/render_details.go b/cmd/render_details.go index 6c7e48a6..64939b91 100644 --- a/cmd/render_details.go +++ b/cmd/render_details.go @@ -192,6 +192,22 @@ func caveatStatLines(g findingGroup) []statLine { return out } +// variablesUnreadableCaveat returns a single ⚠ caveat stat line when the +// settings-variable listing could not be read (nil VariablesData, or Known +// false from a 401/403/null-project). Both variable-control stat builders use +// it so an unreadable listing shows the caveat instead of a bare green "passed" +// — the JSON status is already error via StatusFor, this closes the terminal +// gap. Returns nil when the listing was read authoritatively. +func variablesUnreadableCaveat(result *control.AnalysisResult) []statLine { + if result != nil && result.VariablesData != nil && result.VariablesData.Known { + return nil + } + return []statLine{{ + Label: statCaveatPrefix + " Variables not evaluated", + Value: "CI/CD variables could not be read — token lacks permission or the API returned no data", + }} +} + // renderSkippedControlsSummary prints the "Skipped Controls" section: a // top-level section header followed by each skipped control with its // skip reason. @@ -852,6 +868,22 @@ func buildGitLabControlStats(controlName string, result *control.AnalysisResult, {Label: statActionRefsChecked, Value: fmt.Sprintf("%d", actionRefs)}, {Label: "Mutable Remote Exec Found", Value: fmt.Sprintf("%d", findingsCount)}, } + case "cicdVariablesMustBeProtected": + if lines := variablesUnreadableCaveat(result); lines != nil { + return lines + } + return []statLine{ + {Label: "Variables Checked", Value: fmt.Sprintf("%d", len(result.VariablesData.Variables))}, + {Label: "Unprotected Variables", Value: fmt.Sprintf("%d", findingsCount)}, + } + case "cicdVariablesMustBeMasked": + if lines := variablesUnreadableCaveat(result); lines != nil { + return lines + } + return []statLine{ + {Label: "Variables Checked", Value: fmt.Sprintf("%d", len(result.VariablesData.Variables))}, + {Label: "Unmasked Variables", Value: fmt.Sprintf("%d", findingsCount)}, + } case "branchMustBeProtected": total, toProtect, protected, unprotected := _branchProtectionCounts(result, pc) nonCompliant := 0 diff --git a/configuration/plumberconfig.go b/configuration/plumberconfig.go index bbaaaefe..81968558 100644 --- a/configuration/plumberconfig.go +++ b/configuration/plumberconfig.go @@ -36,6 +36,12 @@ var validControlSchema = map[string][]string{ "allowForcePush", "codeOwnerApprovalRequired", "minMergeAccessLevel", "minPushAccessLevel", }, + "cicdVariablesMustBeProtected": { + "enabled", + }, + "cicdVariablesMustBeMasked": { + "enabled", + }, "pipelineMustNotIncludeHardcodedJobs": { "enabled", }, @@ -258,6 +264,18 @@ type ControlsConfig struct { // BranchMustBeProtected control configuration BranchMustBeProtected *BranchProtectionControlConfig `yaml:"branchMustBeProtected,omitempty"` + // CicdVariablesMustBeProtected control configuration (GitLab only). + // Flags project CI/CD settings variables that are not marked + // protected, so they are exposed to pipelines running on unprotected + // branches (ISSUE-201). Config-free; toggle via `enabled`. + CicdVariablesMustBeProtected *EnabledOnlyControlConfig `yaml:"cicdVariablesMustBeProtected,omitempty"` + + // CicdVariablesMustBeMasked control configuration (GitLab only). + // Flags project CI/CD settings variables that are not masked, so + // their values print verbatim in every job log a project member can + // read (ISSUE-202). Config-free; toggle via `enabled`. + CicdVariablesMustBeMasked *EnabledOnlyControlConfig `yaml:"cicdVariablesMustBeMasked,omitempty"` + // PipelineMustNotIncludeHardcodedJobs control configuration PipelineMustNotIncludeHardcodedJobs *HardcodedJobsControlConfig `yaml:"pipelineMustNotIncludeHardcodedJobs,omitempty"` @@ -1131,6 +1149,20 @@ func (c *PlumberConfig) GetBranchMustBeProtectedConfig() *BranchProtectionContro return c.ControlsFor("gitlab").BranchMustBeProtected } +func (c *PlumberConfig) GetCicdVariablesMustBeProtectedConfig() *EnabledOnlyControlConfig { + if c == nil { + return nil + } + return c.ControlsFor("gitlab").CicdVariablesMustBeProtected +} + +func (c *PlumberConfig) GetCicdVariablesMustBeMaskedConfig() *EnabledOnlyControlConfig { + if c == nil { + return nil + } + return c.ControlsFor("gitlab").CicdVariablesMustBeMasked +} + // 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..d49a6400 100644 --- a/configuration/plumberconfig_test.go +++ b/configuration/plumberconfig_test.go @@ -361,6 +361,8 @@ func TestValidControlNames(t *testing.T) { "actionsMustNotCarryKnownCVEs", "actionsMustNotExecuteMutableRemoteCode", "branchMustBeProtected", + "cicdVariablesMustBeMasked", + "cicdVariablesMustBeProtected", "containerImageMustComeFromAuthorizedSources", "containerImageMustNotUseForbiddenTags", "externalRefsMustNotCollide", diff --git a/configuration/registry.go b/configuration/registry.go index 888043f1..1ecb4699 100644 --- a/configuration/registry.go +++ b/configuration/registry.go @@ -31,6 +31,8 @@ const ( var controlsMeta = map[string]ControlMeta{ // Cross-provider (same control name + rego logic, provider-specific values). "branchMustBeProtected": {Providers: []string{ProviderGitLab, ProviderGitHub}}, + "cicdVariablesMustBeProtected": {Providers: []string{ProviderGitLab}}, + "cicdVariablesMustBeMasked": {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..47d6607f 100644 --- a/configuration/v1_to_v2.go +++ b/configuration/v1_to_v2.go @@ -71,6 +71,8 @@ func controlsConfigIsZero(c ControlsConfig) bool { return c.ContainerImageMustNotUseForbiddenTags == nil && c.ContainerImageMustComeFromAuthorizedSources == nil && c.BranchMustBeProtected == nil && + c.CicdVariablesMustBeProtected == nil && + c.CicdVariablesMustBeMasked == nil && c.PipelineMustNotIncludeHardcodedJobs == nil && c.IncludesMustBeUpToDate == nil && c.IncludesMustNotUseForbiddenVersions == nil && @@ -96,6 +98,8 @@ func controlsConfigEqual(a, b ControlsConfig) bool { return a.ContainerImageMustNotUseForbiddenTags == b.ContainerImageMustNotUseForbiddenTags && a.ContainerImageMustComeFromAuthorizedSources == b.ContainerImageMustComeFromAuthorizedSources && a.BranchMustBeProtected == b.BranchMustBeProtected && + a.CicdVariablesMustBeProtected == b.CicdVariablesMustBeProtected && + a.CicdVariablesMustBeMasked == b.CicdVariablesMustBeMasked && a.PipelineMustNotIncludeHardcodedJobs == b.PipelineMustNotIncludeHardcodedJobs && a.IncludesMustBeUpToDate == b.IncludesMustBeUpToDate && a.IncludesMustNotUseForbiddenVersions == b.IncludesMustNotUseForbiddenVersions && diff --git a/control/catalog.go b/control/catalog.go index 67d8c6fe..d9373dea 100644 --- a/control/catalog.go +++ b/control/catalog.go @@ -52,6 +52,16 @@ func GitLabControls(pc *configuration.PlumberConfig) []ControlEntry { ControlName: "branchMustBeProtected", Skipped: c.BranchMustBeProtected == nil || !c.BranchMustBeProtected.IsEnabled(), }) + entries = append(entries, ControlEntry{ + DisplayName: "CI/CD variables must be protected", + ControlName: "cicdVariablesMustBeProtected", + Skipped: c.CicdVariablesMustBeProtected == nil || !c.CicdVariablesMustBeProtected.IsEnabled(), + }) + entries = append(entries, ControlEntry{ + DisplayName: "CI/CD variables must be masked", + ControlName: "cicdVariablesMustBeMasked", + Skipped: c.CicdVariablesMustBeMasked == nil || !c.CicdVariablesMustBeMasked.IsEnabled(), + }) entries = append(entries, ControlEntry{ DisplayName: "Pipeline must not include hardcoded jobs", ControlName: "pipelineMustNotIncludeHardcodedJobs", @@ -315,6 +325,12 @@ func DisabledControlNames(c *configuration.ControlsConfig) map[string]bool { if cfg := c.BranchMustBeProtected; cfg == nil || !cfg.IsEnabled() { out["branchMustBeProtected"] = true } + if cfg := c.CicdVariablesMustBeProtected; cfg == nil || !cfg.IsEnabled() { + out["cicdVariablesMustBeProtected"] = true + } + if cfg := c.CicdVariablesMustBeMasked; cfg == nil || !cfg.IsEnabled() { + out["cicdVariablesMustBeMasked"] = true + } if cfg := c.PipelineMustNotIncludeHardcodedJobs; cfg == nil || !cfg.IsEnabled() { out["pipelineMustNotIncludeHardcodedJobs"] = true } diff --git a/control/codes.go b/control/codes.go index d3dabe56..acc92f86 100644 --- a/control/codes.go +++ b/control/codes.go @@ -59,6 +59,10 @@ const ( // Issue codes for CI/CD variable controls (2xx) const ( + // ISSUE-201: CI/CD settings variable is not protected (exposed to pipelines on unprotected branches) + CodeCicdVariableUnprotected ErrorCode = "ISSUE-201" + // ISSUE-202: CI/CD settings variable is not masked (its value prints in job logs) + CodeCicdVariableUnmasked ErrorCode = "ISSUE-202" // ISSUE-203: Pipeline enables CI debug trace (CI_DEBUG_TRACE or CI_DEBUG_SERVICES) CodeDebugTraceEnabled ErrorCode = "ISSUE-203" // ISSUE-204: Unsafe variable expansion in shell re-interpretation context (eval, sh -c, etc.) @@ -401,6 +405,24 @@ var errorCodeRegistry = map[ErrorCode]ErrorCodeInfo{ }, // CI/CD variable controls (2xx) + CodeCicdVariableUnprotected: { + Code: CodeCicdVariableUnprotected, + Severity: SeverityMedium, + Title: "CI/CD settings variable is not protected", + Description: "A project CI/CD settings variable is not marked protected, so it is injected into pipelines running on unprotected branches. Anyone who can push to such a branch can exfiltrate its value.", + Remediation: "Mark the variable protected in Settings > CI/CD > Variables so it is only exposed to pipelines on protected branches and tags.", + DocURL: docsBaseURL + string(CodeCicdVariableUnprotected), + ControlName: "cicdVariablesMustBeProtected", + }, + CodeCicdVariableUnmasked: { + Code: CodeCicdVariableUnmasked, + Severity: SeverityMedium, + Title: "CI/CD settings variable is not masked", + Description: "A project CI/CD settings variable is not masked, so its value prints verbatim in every job log a project member can read.", + Remediation: "Enable masking in Settings > CI/CD > Variables. GitLab requires a value of at least 8 characters; restructure the secret if it cannot be masked by length.", + DocURL: docsBaseURL + string(CodeCicdVariableUnmasked), + ControlName: "cicdVariablesMustBeMasked", + }, CodeDebugTraceEnabled: { Code: CodeDebugTraceEnabled, Severity: SeverityCritical, diff --git a/control/degraded.go b/control/degraded.go index c35eceb6..545cb20c 100644 --- a/control/degraded.go +++ b/control/degraded.go @@ -63,6 +63,14 @@ func markDegraded(result *AnalysisResult, reason string) { // status classification, so both writers build their strings from it. const degradedReasonBranchProtectionPrefix = "branch protection could not be fetched" +// degradedReasonVariablesPrefix is the shared prefix of the settings-variable +// fetch degraded reason. Like the branch-protection prefix, it is the +// compile-time contract between task.go (the writer) and StatusFor's +// degradedReasonIsVariables classifier, so a network failure fetching the +// variables listing degrades only the two variable controls rather than +// flipping every unrelated CI-file control to error. +const degradedReasonVariablesPrefix = "CI/CD variables could not be fetched" + // degradedReasonsFromGitHubCollection builds the human-readable list of // collection failures behind a degraded GitHub run (#220). partialCount // is the number of workflow files that could not be fetched/parsed and diff --git a/control/mrcomment.go b/control/mrcomment.go index dabb87e0..3cbf4ca9 100644 --- a/control/mrcomment.go +++ b/control/mrcomment.go @@ -252,6 +252,8 @@ func writeIssueDetails(b *strings.Builder, result *AnalysisResult) { {"containerImageMustNotUseForbiddenTags", "Container images must not use forbidden tags"}, {"containerImageMustComeFromAuthorizedSources", "Container images must come from authorized sources"}, {"branchMustBeProtected", "Branch must be protected"}, + {"cicdVariablesMustBeProtected", "CI/CD variables must be protected"}, + {"cicdVariablesMustBeMasked", "CI/CD variables must be masked"}, {"pipelineMustNotIncludeHardcodedJobs", "Pipeline must not include hardcoded jobs"}, {"includesMustBeUpToDate", "Includes must be up to date"}, {"includesMustNotUseForbiddenVersions", "Includes must not use forbidden versions"}, diff --git a/control/status.go b/control/status.go index bcf0b01e..b1c53b69 100644 --- a/control/status.go +++ b/control/status.go @@ -25,6 +25,15 @@ func degradedReasonIsBranchProtection(reason string) bool { return strings.HasPrefix(reason, degradedReasonBranchProtectionPrefix) } +// degradedReasonIsVariables classifies a DegradedReasons entry as the +// settings-variable fetch failure. Carved out the same way as branch +// protection so a variables network failure does not flip every unrelated +// CI-file control to error — only the two variable controls report it (via +// their VariablesData.Known check below). +func degradedReasonIsVariables(reason string) bool { + return strings.HasPrefix(reason, degradedReasonVariablesPrefix) +} + // StatusFor derives a control's evaluation status for a run. // // Order matters: findings trump degradation — when a control found real @@ -88,11 +97,25 @@ func StatusFor(e ControlEntry, result *AnalysisResult, findingCount int) string } return StatusPassed } + if e.ControlName == "cicdVariablesMustBeProtected" || e.ControlName == "cicdVariablesMustBeMasked" { + // Settings-variable controls are independent of the CI file: they + // evaluate the project's settings variables, not the pipeline, so + // the CiMissing / CiValid check below does not apply. The listing + // is authoritative only when the collection ran and succeeded + // (VariablesData set, Known=true). A nil VariablesData (the + // control's collection never ran) or Known=false (a 401/403 from a + // token that cannot read variables) means the control never truly + // evaluated: an empty findings list here must not read as a pass. + if result.VariablesData == nil || !result.VariablesData.Known { + return StatusError + } + return StatusPassed + } if result.CiMissing || !result.CiValid { return StatusError } for _, r := range result.DegradedReasons { - if !degradedReasonIsBranchProtection(r) { + if !degradedReasonIsBranchProtection(r) && !degradedReasonIsVariables(r) { return StatusError } } diff --git a/control/status_test.go b/control/status_test.go index d223e824..5a723363 100644 --- a/control/status_test.go +++ b/control/status_test.go @@ -9,6 +9,7 @@ import ( func TestStatusFor(t *testing.T) { content := ControlEntry{ControlName: "actionsMustBePinnedByCommitSha"} branch := ControlEntry{ControlName: "branchMustBeProtected"} + variables := ControlEntry{ControlName: "cicdVariablesMustBeProtected"} healthy := &AnalysisResult{CiValid: true} cases := []struct { @@ -34,6 +35,11 @@ func TestStatusFor(t *testing.T) { {"branch control on github ignores content-only degradation", branch, &AnalysisResult{CiValid: true, GitHubStats: &GitHubAnalysisStats{}, DegradedReasons: []string{"2 workflow file(s) could not be fetched and were skipped"}}, 0, StatusPassed}, {"branch control on gitlab ignores content-only degradation when its own collection ran", branch, &AnalysisResult{CiValid: true, ProtectionData: &gitlab.GitlabProtectionAnalysisData{}, DegradedReasons: []string{"2 include(s) could not be resolved; their jobs were not analysed"}}, 0, StatusPassed}, {"nil result defaults to passed (hand-built test fixtures)", content, nil, 0, StatusPassed}, + {"variable control passes when the listing was read and clean", variables, &AnalysisResult{CiValid: true, VariablesData: &gitlab.GitlabVariablesAnalysisData{Known: true}}, 0, StatusPassed}, + {"variable control ignores missing CI config (settings-independent) when the listing was read", variables, &AnalysisResult{CiMissing: true, VariablesData: &gitlab.GitlabVariablesAnalysisData{Known: true}}, 0, StatusPassed}, + {"variable control errors when its collection never ran", variables, &AnalysisResult{CiValid: true}, 0, StatusError}, + {"variable control errors on an unreadable listing (401/403, Known=false): empty findings are not a pass", variables, &AnalysisResult{CiValid: true, VariablesData: &gitlab.GitlabVariablesAnalysisData{Known: false}}, 0, StatusError}, + {"variable control with findings is failed regardless of CI state", variables, &AnalysisResult{CiValid: true, VariablesData: &gitlab.GitlabVariablesAnalysisData{Known: true}}, 3, StatusFailed}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/control/task.go b/control/task.go index e1035120..41a301f4 100644 --- a/control/task.go +++ b/control/task.go @@ -34,6 +34,26 @@ const opaEvaluateTimeout = 2 * time.Minute const controlBranchMustBeProtected = "branchMustBeProtected" const controlMutableRemoteExec = "actionsMustNotExecuteMutableRemoteCode" +// controlCicdVariablesMustBeProtected / ...Masked are the two .plumber.yaml +// control keys the task flow references directly, to decide whether to fetch +// the project's settings-variable listing (an extra API call) before invoking +// the Rego engine. Both share one collector (CollectGitlabVariables). +const controlCicdVariablesMustBeProtected = "cicdVariablesMustBeProtected" +const controlCicdVariablesMustBeMasked = "cicdVariablesMustBeMasked" + +// cicdVariableControlEnabled reports whether either settings-variable control +// is active for this run, so the variable listing is fetched only when a +// control needs it. +func cicdVariableControlEnabled(conf *configuration.Configuration) bool { + if p := conf.PlumberConfig.GetCicdVariablesMustBeProtectedConfig(); p != nil && p.IsEnabled() && shouldRunControl(controlCicdVariablesMustBeProtected, conf) { + return true + } + if m := conf.PlumberConfig.GetCicdVariablesMustBeMaskedConfig(); m != nil && m.IsEnabled() && shouldRunControl(controlCicdVariablesMustBeMasked, conf) { + return true + } + return false +} + // shouldScanMutableExec reports whether the collector should fetch and // scan action source for actionsMustNotExecuteMutableRemoteCode // (ISSUE-714/715). The scan is expensive (up to ~7 sequential HTTP @@ -119,6 +139,7 @@ func runRegoEngine( originData *gitlab.GitlabPipelineOriginData, imageData *gitlab.GitlabPipelineImageData, protectionData *gitlab.GitlabProtectionAnalysisData, + variablesData *gitlab.GitlabVariablesAnalysisData, ) []opaengine.Finding { pipeline := gitlab.ToNormalizedPipeline( conf.ProjectPath, @@ -127,6 +148,7 @@ func runRegoEngine( originData, imageData, protectionData, + variablesData, ) return evaluatePolicies(l, conf, "gitlab", pipeline) } @@ -646,11 +668,31 @@ func RunAnalysis(conf *configuration.Configuration) (*AnalysisResult, error) { } } + // Fetch settings-variable metadata when either variable control is + // enabled — the Rego policies need the protected/masked flags, and the + // (extra API call) listing is fetched only when a control needs it. The + // variable values never reach the IR (per the #370 sensitivity tiers). + var variablesData *gitlab.GitlabVariablesAnalysisData + if cicdVariableControlEnabled(conf) { + reportProgress(conf, 10, analysisStepCount, "Checking CI/CD variables") + var vErr error + variablesData, vErr = gitlab.CollectGitlabVariables(conf.ProjectPath, conf.GitlabToken, conf) + if vErr != nil && isNetworkError(vErr) { + // A transient network failure fetching variables degrades the run + // (exit 3), matching the branch/image collectors (#220). A + // permission failure (401/403 or a null project) is NOT network, so + // it stays a plain not-evaluable — variablesData.Known is false and + // StatusFor reports it without failing an otherwise-complete run. + markDegraded(result, degradedReasonVariablesPrefix+" (network or timeout)") + } + } + // Rego/OPA rule engine evaluation — the single authoritative // compliance path (the legacy Go controls were retired in // docs/REFACTOR_MULTI_PROVIDER.md §8 Phase A). - result.Findings = runRegoEngine(l, conf, project, pipelineOriginData, pipelineImageData, protectionData) + result.Findings = runRegoEngine(l, conf, project, pipelineOriginData, pipelineImageData, protectionData, variablesData) result.ProtectionData = protectionData + result.VariablesData = variablesData reportProgress(conf, analysisStepCount, analysisStepCount, "Analysis complete") diff --git a/control/task_variables_gate_test.go b/control/task_variables_gate_test.go new file mode 100644 index 00000000..f370c1a9 --- /dev/null +++ b/control/task_variables_gate_test.go @@ -0,0 +1,104 @@ +package control + +import ( + "testing" + + "github.com/getplumber/plumber/configuration" + "github.com/getplumber/plumber/gitlab" +) + +// cicdVariableControlEnabled decides whether the extra settings-variable API +// call runs, and therefore whether the two controls can produce findings at +// all. A false negative leaves variablesData nil and the controls report +// not-evaluable (never evaluate); a false positive makes an unnecessary API +// call every run. Mirrors TestShouldScanMutableExec for the sibling gate. +func TestCicdVariableControlEnabled(t *testing.T) { + boolPtr := func(b bool) *bool { return &b } + on := &configuration.EnabledOnlyControlConfig{Enabled: boolPtr(true)} + off := &configuration.EnabledOnlyControlConfig{Enabled: boolPtr(false)} + + cfgWith := func(protected, masked *configuration.EnabledOnlyControlConfig) *configuration.Configuration { + return &configuration.Configuration{PlumberConfig: &configuration.PlumberConfig{ + GitLab: &configuration.ProviderConfig{Controls: configuration.ControlsConfig{ + CicdVariablesMustBeProtected: protected, + CicdVariablesMustBeMasked: masked, + }}, + }} + } + + t.Run("nil PlumberConfig -> false", func(t *testing.T) { + if cicdVariableControlEnabled(&configuration.Configuration{}) { + t.Fatal("expected false when PlumberConfig is nil") + } + }) + t.Run("both absent -> false", func(t *testing.T) { + if cicdVariableControlEnabled(cfgWith(nil, nil)) { + t.Fatal("expected false when neither control is configured") + } + }) + t.Run("both disabled -> false", func(t *testing.T) { + if cicdVariableControlEnabled(cfgWith(off, off)) { + t.Fatal("expected false when both controls are disabled") + } + }) + t.Run("protected only -> true", func(t *testing.T) { + if !cicdVariableControlEnabled(cfgWith(on, nil)) { + t.Fatal("expected true when the protected control is enabled") + } + }) + t.Run("masked only -> true", func(t *testing.T) { + if !cicdVariableControlEnabled(cfgWith(nil, on)) { + t.Fatal("expected true when the masked control is enabled") + } + }) + t.Run("--skip-controls excludes both -> false", func(t *testing.T) { + conf := cfgWith(on, on) + conf.SkipControlsFilter = []string{controlCicdVariablesMustBeProtected, controlCicdVariablesMustBeMasked} + if cicdVariableControlEnabled(conf) { + t.Fatal("expected false when both controls are in --skip-controls") + } + }) + t.Run("--controls omitting both -> false", func(t *testing.T) { + conf := cfgWith(on, on) + conf.ControlsFilter = []string{"branchMustBeProtected"} + if cicdVariableControlEnabled(conf) { + t.Fatal("expected false when --controls omits both variable controls") + } + }) +} + +// TestStatusFor_VariablesDegradedCarveOut pins the fix for the "variables +// network failure flips every unrelated control to error" blocker: the +// variables degraded reason is carved out of StatusFor's degrade loop the same +// way branch protection is, so only the two variable controls report it. +func TestStatusFor_VariablesDegradedCarveOut(t *testing.T) { + varsReason := degradedReasonVariablesPrefix + " (network or timeout)" + result := &AnalysisResult{ + CiValid: true, + DegradedReasons: []string{varsReason}, + VariablesData: &gitlab.GitlabVariablesAnalysisData{Known: false}, + } + unrelated := ControlEntry{ControlName: "pipelineMustNotUseDockerInDocker"} + if got := StatusFor(unrelated, result, 0); got != StatusPassed { + t.Fatalf("a variables degrade must leave an unrelated CI-file control passed, got %q", got) + } + varsCtrl := ControlEntry{ControlName: "cicdVariablesMustBeProtected"} + if got := StatusFor(varsCtrl, result, 0); got != StatusError { + t.Fatalf("the variables control with Known=false must report error (not-evaluable), got %q", got) + } + // A non-carved-out degrade still flips unrelated controls to error. + other := &AnalysisResult{CiValid: true, DegradedReasons: []string{"something unrelated failed"}} + if got := StatusFor(unrelated, other, 0); got != StatusError { + t.Fatalf("a non-carved degrade must still flip unrelated controls to error, got %q", got) + } +} + +// TestDegradedReasonIsVariables pins the classifier prefix contract. +func TestDegradedReasonIsVariables(t *testing.T) { + if !degradedReasonIsVariables(degradedReasonVariablesPrefix + " (network or timeout)") { + t.Fatal("classifier must match the variables degraded-reason prefix") + } + if degradedReasonIsVariables(degradedReasonBranchProtectionPrefix + " (network or timeout)") { + t.Fatal("classifier must not match the branch-protection reason") + } +} diff --git a/control/types.go b/control/types.go index 898f7a4d..c2a040c2 100644 --- a/control/types.go +++ b/control/types.go @@ -55,6 +55,11 @@ type AnalysisResult struct { PipelineImageData *gitlab.GitlabPipelineImageData `json:"-"` PipelineOriginData *gitlab.GitlabPipelineOriginData `json:"-"` ProtectionData *gitlab.GitlabProtectionAnalysisData `json:"-"` + // VariablesData records the settings-variable collection for the + // cicdVariablesMustBe* controls. Set only after the collection ran; + // nil (never ran) or Known=false (401/403) makes those controls + // report not-evaluable rather than a false pass (see StatusFor). + VariablesData *gitlab.GitlabVariablesAnalysisData `json:"-"` // GitHubStats holds per-control denominators computed from the // GitHub IR after a GitHub analysis. Used by the GitHub renderer diff --git a/defaultConfig/.plumber.yaml b/defaultConfig/.plumber.yaml index 9c5fcb4d..65e57109 100644 --- a/defaultConfig/.plumber.yaml +++ b/defaultConfig/.plumber.yaml @@ -209,6 +209,31 @@ gitlab: - redhat/* # = docker.io/redhat/* (bare ref, unresolved-$VAR case) - opensuse/* # = docker.io/opensuse/* (bare ref, unresolved-$VAR case) # =========================================== + # CI/CD variables must be protected + # =========================================== + # Flags project CI/CD settings variables (Settings > CI/CD > + # Variables) that are not marked protected, so they are exposed to + # pipelines on unprotected branches that any developer can push to. + # Requires a GitLab API token with read access to variables; on a + # 401/403 the control reports not-evaluable rather than a false pass. + # + # Ships disabled: enable it once you have reviewed which settings + # variables are intentionally unprotected. + cicdVariablesMustBeProtected: + # Set to true to enable this control + enabled: false + # =========================================== + # CI/CD variables must be masked + # =========================================== + # Flags project CI/CD settings variables that are not masked, so + # their values print verbatim in every job log a project member can + # read. GitLab cannot mask values shorter than 8 characters; such + # variables are still flagged because the exposure is real. + # + # Ships disabled: enable it after reviewing your variables. + cicdVariablesMustBeMasked: + # Set to true to enable this control + enabled: false # Branch must be protected # =========================================== # Checks that repository branches have proper protection settings. diff --git a/finding/identity/declarations.go b/finding/identity/declarations.go index a03bf30b..29db8f24 100644 --- a/finding/identity/declarations.go +++ b/finding/identity/declarations.go @@ -81,6 +81,10 @@ var declarations = map[string][]string{ "ISSUE-102": {"file", "job", "link"}, // Image not pinned by digest: keyed on the image repository (registry/name, no tag) so a tag bump on a still-digestless image does not re-key. "ISSUE-103": {"file", "job", "imageRepo"}, + // CI/CD settings variable not protected: keyed on the variable identity (settings-level, no file or job), mirroring the platform IdOnly (Name/Type/Environment). + "ISSUE-201": {"variableName", "variableType", "environment"}, + // CI/CD settings variable not masked: keyed on the variable identity (settings-level, no file or job), same shape as ISSUE-201. + "ISSUE-202": {"variableName", "variableType", "environment"}, // CI debug trace enabled: keyed on the variable name. "ISSUE-203": {"file", "job", "variableName"}, // Unsafe variable expansion: keyed on the variable name. diff --git a/finding/identity/identity_test.go b/finding/identity/identity_test.go index 76ccdc35..0fbb5e75 100644 --- a/finding/identity/identity_test.go +++ b/finding/identity/identity_test.go @@ -320,6 +320,8 @@ func TestFingerprint_MatchesTheShippedValues(t *testing.T) { func TestDeclarations_EveryCodeFingerprintIsPinned(t *testing.T) { golden := map[string]string{ "ISSUE-101": "042bff0ebb1d89ee", + "ISSUE-201": "ded1b4bbe17f4b90", + "ISSUE-202": "df6d1d45432e1cb5", "ISSUE-102": "32b497995ae3be65", "ISSUE-103": "ae7c300b437b0bb0", "ISSUE-203": "0ebe5475110580fc", diff --git a/gitlab/dataCollectionGitlabPipelineImage.go b/gitlab/dataCollectionGitlabPipelineImage.go index 8f3ad9dc..a2daf65a 100644 --- a/gitlab/dataCollectionGitlabPipelineImage.go +++ b/gitlab/dataCollectionGitlabPipelineImage.go @@ -1,6 +1,7 @@ package gitlab import ( + "errors" "fmt" "strings" @@ -727,8 +728,17 @@ func (dc *GitlabPipelineImageDataCollection) Run(project *ProjectInfo, token str // Get project variables projectVarsResult, err := GetGitlabProjectVariables(project.Path, token, conf.GitlabURL, conf) if err != nil { - l.WithError(err).Error("Unable to retrieve project variables") - return data, metrics, err + if errors.Is(err, ErrProjectVariablesUnreadable) { + // The token cannot read project variables; they are only used here + // to resolve image-ref placeholders like $CI_REGISTRY, so proceed + // without them rather than failing the whole image collection. This + // preserves the pre-#418 graceful degradation for image analysis. + l.WithError(err).Warn("project variables not readable; image analysis proceeds without them") + projectVarsResult = nil + } else { + l.WithError(err).Error("Unable to retrieve project variables") + return data, metrics, err + } } data.ProjectVars = ConvertCICDVariableToMap(projectVarsResult) l.WithField("projectVarKeys", GetMapKeys(data.ProjectVars)).Debug("Project vars found") diff --git a/gitlab/dataCollectionGitlabVariables.go b/gitlab/dataCollectionGitlabVariables.go new file mode 100644 index 00000000..15fd176f --- /dev/null +++ b/gitlab/dataCollectionGitlabVariables.go @@ -0,0 +1,55 @@ +package gitlab + +import ( + "github.com/getplumber/plumber/configuration" + "github.com/sirupsen/logrus" +) + +// GitlabVariablesAnalysisData holds the project's settings CI/CD variables +// (GitLab: Settings > CI/CD > Variables) with their security flags, for the +// cicdVariablesMustBeProtected / cicdVariablesMustBeMasked controls. +// +// Known records whether the listing was read authoritatively. A 401/403 (or +// any other settings-API failure) leaves it false, so the controls report +// not-evaluable rather than a false pass: a token that cannot read the +// variables must not make an unprotected variable look protected (#418). +type GitlabVariablesAnalysisData struct { + Variables []CICDVariable + Known bool +} + +// CollectGitlabVariables fetches the project's settings CI/CD variables with +// their protected/masked flags. Any fetch failure — most importantly a +// 401/403 from a token without the variable-read scope — is a definitive +// "cannot evaluate", never a false pass: Variables stays empty and Known +// stays false. The variable values are fetched but never projected onto the +// IR (see gitlab_ir.go::buildSettingsVariables), per the #370 +// variable-sensitivity tiers. +// +// The fetch error is also returned (alongside the always-non-nil data) so the +// caller can distinguish a transient network failure — which must degrade the +// run (exit 3), like the branch/image collectors (#220) — from a definitive +// permission failure (401/403 or a null project), which stays a plain +// not-evaluable. Known is false in both cases. +func CollectGitlabVariables(fullPath, token string, conf *configuration.Configuration) (*GitlabVariablesAnalysisData, error) { + l := logrus.WithFields(logrus.Fields{ + "platform": "gitlab", + "action": "CollectGitlabVariables", + "project": fullPath, + }) + vars, err := GetGitlabProjectVariables(fullPath, token, conf.GitlabURL, conf) + if err != nil { + l.WithError(err).Warn("settings-variable listing unreadable; cicdVariablesMustBe* will report not-evaluable") + return &GitlabVariablesAnalysisData{Known: false}, err + } + // The settings controls only need each variable's flags (protected / masked + // / type / environmentScope), never its value — image resolution uses a + // separate fetch. Blank the value so a settings-variable secret is never + // held on this data or risked in any downstream serialization. VariablesData + // is already json:"-" and the IR carries no Value field; this is defense in + // depth at the collection boundary. + for i := range vars { + vars[i].Value = "" + } + return &GitlabVariablesAnalysisData{Variables: vars, Known: true}, nil +} diff --git a/gitlab/dataCollectionGitlabVariables_test.go b/gitlab/dataCollectionGitlabVariables_test.go new file mode 100644 index 00000000..262bd8b8 --- /dev/null +++ b/gitlab/dataCollectionGitlabVariables_test.go @@ -0,0 +1,146 @@ +package gitlab + +import ( + "errors" + "net/http" + "net/http/httptest" + "testing" + + "github.com/getplumber/plumber/configuration" +) + +// TestCollectGitlabVariables_UnreadableIsNotEvaluable: a failed fetch (here an +// HTTP 401) maps to Known=false so an unreadable settings API reports +// not-evaluable, never a false pass (#418), and the error is returned so the +// caller can classify it (network -> degrade vs permission -> not-evaluable). +func TestCollectGitlabVariables_UnreadableIsNotEvaluable(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + })) + defer srv.Close() + + conf := &configuration.Configuration{GitlabURL: srv.URL} + data, err := CollectGitlabVariables("group/project", "bad-token", conf) + if err == nil { + t.Fatal("expected a non-nil error on a failed fetch so the caller can classify it") + } + if data == nil || data.Known { + t.Fatalf("an unreadable settings API must yield Known=false, got %+v", data) + } + if len(data.Variables) != 0 { + t.Fatalf("expected zero variables on a failed fetch, got %d", len(data.Variables)) + } +} + +// TestCollectGitlabVariables_NullProjectIsNotEvaluable covers the +// under-privileged case: HTTP 200 (no transport error) with the GraphQL +// `project` field null. Must yield Known=false, and the error must wrap +// ErrProjectVariablesUnreadable so the image collector can tolerate it via +// errors.Is while the settings controls treat it as not-evaluable. +func TestCollectGitlabVariables_NullProjectIsNotEvaluable(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"data":{"project":null}}`)) + })) + defer srv.Close() + + conf := &configuration.Configuration{GitlabURL: srv.URL} + data, err := CollectGitlabVariables("group/project", "under-privileged", conf) + if data.Known { + t.Fatal("a null project (HTTP 200, under-privileged token) must yield Known=false") + } + if !errors.Is(err, ErrProjectVariablesUnreadable) { + t.Fatalf("null-project error must wrap ErrProjectVariablesUnreadable (the image collector relies on it via errors.Is), got %v", err) + } +} + +// TestGetGitlabProjectVariables_SuccessMapping exercises the success path: a +// populated ciVariables node list maps to []CICDVariable, and the GraphQL enum +// variableType (ENV_VAR / FILE) is normalised to lower case so the masked +// rule's file exclusion and the ISSUE-201/202 identity value stay consistent +// with the fixtures and the REST convention. The hand-built IR fixtures +// elsewhere already carry lower-case types, so only this test catches a dropped +// ToLower or a field mix-up in the mapping. +func TestGetGitlabProjectVariables_SuccessMapping(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"data":{"project":{"ciVariables":{"pageInfo":{"hasNextPage":false,"endCursor":""},"nodes":[ + {"key":"AWS_KEY","value":"x","variableType":"ENV_VAR","masked":true,"protected":false,"hidden":false,"environmentScope":"*"}, + {"key":"KUBECONFIG","value":"y","variableType":"FILE","masked":false,"protected":true,"hidden":false,"environmentScope":"production"} + ]}}}}`)) + })) + defer srv.Close() + + conf := &configuration.Configuration{GitlabURL: srv.URL} + vars, err := GetGitlabProjectVariables("group/project", "tok", conf.GitlabURL, conf) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(vars) != 2 { + t.Fatalf("want 2 variables, got %d", len(vars)) + } + if v := vars[0]; v.Name != "AWS_KEY" || v.Type != "env_var" || !v.Masked || v.Protected { + t.Fatalf("var 0 mapping mismatch (type must be lower-cased env_var): %+v", v) + } + if v := vars[1]; v.Name != "KUBECONFIG" || v.Type != "file" || v.Masked || !v.Protected || v.Environment != "production" { + t.Fatalf("var 1 mapping mismatch (type must be lower-cased file): %+v", v) + } +} + +// TestGetGitlabProjectVariables_Paginates pins cursor pagination: a variable +// returned only on page 2 (after following endCursor) must appear in the +// result, so a project with more than one page of variables is fully scanned. +func TestGetGitlabProjectVariables_Paginates(t *testing.T) { + call := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + call++ + if call == 1 { + _, _ = w.Write([]byte(`{"data":{"project":{"ciVariables":{"pageInfo":{"hasNextPage":true,"endCursor":"CURSOR1"},"nodes":[ + {"key":"PAGE1_VAR","value":"a","variableType":"ENV_VAR","masked":false,"protected":false,"hidden":false,"environmentScope":"*"} + ]}}}}`)) + return + } + _, _ = w.Write([]byte(`{"data":{"project":{"ciVariables":{"pageInfo":{"hasNextPage":false,"endCursor":""},"nodes":[ + {"key":"PAGE2_VAR","value":"b","variableType":"FILE","masked":false,"protected":true,"hidden":false,"environmentScope":"prod"} + ]}}}}`)) + })) + defer srv.Close() + + conf := &configuration.Configuration{GitlabURL: srv.URL} + vars, err := GetGitlabProjectVariables("group/project", "tok", conf.GitlabURL, conf) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(vars) != 2 { + t.Fatalf("expected 2 variables across 2 pages, got %d", len(vars)) + } + if vars[1].Name != "PAGE2_VAR" { + t.Errorf("the page-2 variable was not fetched (pagination broken): %+v", vars) + } +} + +// TestCollectGitlabVariables_BlanksValues pins that the settings-control path +// strips every variable value (image resolution uses a separate fetch), so a +// secret value is never held on VariablesData even though the GraphQL query +// still selects it. +func TestCollectGitlabVariables_BlanksValues(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"data":{"project":{"ciVariables":{"pageInfo":{"hasNextPage":false,"endCursor":""},"nodes":[ + {"key":"SECRET","value":"PLANTED_SECRET","variableType":"ENV_VAR","masked":false,"protected":false,"hidden":false,"environmentScope":"*"} + ]}}}}`)) + })) + defer srv.Close() + + conf := &configuration.Configuration{GitlabURL: srv.URL} + data, err := CollectGitlabVariables("group/project", "tok", conf) + if err != nil || !data.Known || len(data.Variables) != 1 { + t.Fatalf("expected a clean success with 1 variable, got Known=%v n=%d err=%v", data.Known, len(data.Variables), err) + } + if data.Variables[0].Value != "" { + t.Errorf("settings-path variable value must be blanked, got %q", data.Variables[0].Value) + } +} diff --git a/gitlab/gitlab_ir.go b/gitlab/gitlab_ir.go index 4c5d8f7f..44b70d04 100644 --- a/gitlab/gitlab_ir.go +++ b/gitlab/gitlab_ir.go @@ -33,6 +33,7 @@ func ToNormalizedPipeline( origin *GitlabPipelineOriginData, images *GitlabPipelineImageData, protection *GitlabProtectionAnalysisData, + variables *GitlabVariablesAnalysisData, ) *ir.NormalizedPipeline { pipeline := &ir.NormalizedPipeline{ Provider: ir.ProviderGitLab, @@ -47,6 +48,7 @@ func ToNormalizedPipeline( pipeline.Includes = buildIncludes(origin, ciConfigPath) pipeline.Jobs = buildJobs(origin, imagesByJob, ciConfigPath, pipeline.Includes) pipeline.Branches = buildBranches(protection) + pipeline.SettingsVariables, pipeline.SettingsVariablesKnown = buildSettingsVariables(variables) if origin != nil && origin.MergedConf != nil { if globals := extractGitLabVariables(origin.MergedConf.GlobalVariables); len(globals) > 0 { pipeline.GlobalVariables = globals @@ -61,6 +63,29 @@ func ToNormalizedPipeline( return pipeline } +// buildSettingsVariables projects the collected settings CI/CD variables onto +// the IR, carrying identity and flags only — never the value (per the #370 +// variable-sensitivity tiers). The second return is SettingsVariablesKnown: +// false when the listing was unreadable (nil data, or a 401/403 the collector +// recorded as Known=false), so a control keyed on these variables reports +// not-evaluable rather than a false pass. +func buildSettingsVariables(variables *GitlabVariablesAnalysisData) ([]ir.SettingsVariable, bool) { + if variables == nil { + return nil, false + } + out := make([]ir.SettingsVariable, 0, len(variables.Variables)) + for _, v := range variables.Variables { + out = append(out, ir.SettingsVariable{ + Name: v.Name, + Type: v.Type, + Environment: v.Environment, + Protected: v.Protected, + Masked: v.Masked, + }) + } + return out, variables.Known +} + // 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..2184d013 100644 --- a/gitlab/gitlab_ir_test.go +++ b/gitlab/gitlab_ir_test.go @@ -6,8 +6,42 @@ import ( "github.com/getplumber/plumber/internal/ir" ) +func TestBuildSettingsVariables(t *testing.T) { + // nil collector data -> not known, no variables, so a control keyed on + // these reports not-evaluable rather than a false pass. + if got, known := buildSettingsVariables(nil); got != nil || known { + t.Fatalf("nil data: got %v known %v, want nil/false", got, known) + } + + // An unreadable listing (a 401/403 the collector recorded) stays + // known=false even if variables are somehow present. + if _, known := buildSettingsVariables(&GitlabVariablesAnalysisData{Known: false}); known { + t.Fatal("unreadable listing must report known=false") + } + + // A known listing projects identity + flags. The value is fetched but + // never projected: ir.SettingsVariable has no Value field, so the secret + // cannot leak through the IR. + data := &GitlabVariablesAnalysisData{ + Known: true, + Variables: []CICDVariable{ + {Name: "AWS_KEY", Type: "env_var", Environment: "*", Protected: false, Masked: true, Value: "supersecretvalue"}, + }, + } + got, known := buildSettingsVariables(data) + if !known { + t.Fatal("known listing must report known=true") + } + if len(got) != 1 { + t.Fatalf("want 1 projected variable, got %d", len(got)) + } + if v := got[0]; v.Name != "AWS_KEY" || v.Type != "env_var" || v.Environment != "*" || v.Protected || !v.Masked { + t.Fatalf("projection mismatch: %+v", v) + } +} + func TestToNormalizedPipeline_Empty(t *testing.T) { - pipeline := ToNormalizedPipeline("group/project", "main", "", nil, nil, nil) + pipeline := ToNormalizedPipeline("group/project", "main", "", nil, nil, nil, nil) if pipeline.Provider != ir.ProviderGitLab { t.Fatalf("expected provider gitlab, got %q", pipeline.Provider) } @@ -37,7 +71,7 @@ func TestToNormalizedPipeline_JobsAndImages(t *testing.T) { }, } - pipeline := ToNormalizedPipeline("grp/proj", "main", "", origin, images, nil) + pipeline := ToNormalizedPipeline("grp/proj", "main", "", origin, images, nil, nil) if got := len(pipeline.Jobs); got != 3 { t.Fatalf("expected 3 jobs, got %d", got) @@ -71,7 +105,7 @@ func TestToNormalizedPipeline_NilJobInMap(t *testing.T) { }, } - pipeline := ToNormalizedPipeline("grp/proj", "main", "", origin, nil, nil) + pipeline := ToNormalizedPipeline("grp/proj", "main", "", origin, nil, nil, nil) if got := len(pipeline.Jobs); got != 1 { t.Fatalf("expected 1 job (nil entry skipped), got %d", got) } diff --git a/gitlab/request.go b/gitlab/request.go index d5004b62..b0520155 100644 --- a/gitlab/request.go +++ b/gitlab/request.go @@ -2,13 +2,25 @@ package gitlab import ( "context" + "errors" "fmt" + "strings" "github.com/getplumber/plumber/configuration" "github.com/machinebox/graphql" "github.com/sirupsen/logrus" ) +// ErrProjectVariablesUnreadable signals that GetGitlabProjectVariables got a +// well-formed HTTP 200 whose GraphQL `project`/`ciVariables` resolved to null — +// the token authenticated but lacks the role to read CI/CD variables +// (Maintainer+ / admin_cicd_variables). Callers that must never turn +// "unreadable" into a silent empty pass (the settings-variable controls, #418) +// treat any error as not-evaluable. Callers that only want the values +// opportunistically (image-ref resolution) check errors.Is for this sentinel +// and proceed with no project variables instead of failing the whole run. +var ErrProjectVariablesUnreadable = errors.New("project CI/CD variables not readable (insufficient token permissions)") + // GetGitlabProjectInheritedVariables returns all project inherited variables func GetGitlabProjectInheritedVariables(fullPath string, token string, instanceUrl string, conf *configuration.Configuration) ([]CICDVariable, error) { l := logrus.WithFields(logrus.Fields{ @@ -281,9 +293,16 @@ func GetGitlabProjectVariables(fullPath string, token string, instanceUrl string EndCursor string `json:"endCursor"` } `json:"pageInfo"` } + // Project and CiVariables are pointers so a GraphQL `project: null` or + // `ciVariables: null` is distinguishable from an empty list. GitLab returns + // null there (HTTP 200, no transport error) when the token authenticates but + // lacks the role to read CI/CD variables (Maintainer+ / admin_cicd_variables), + // or when the project is not visible. A value struct would silently + // deserialize that to zero variables — a false "no variables" pass. Detecting + // null and erroring keeps the not-evaluable guarantee honest (#418). type response struct { - Project struct { - CiVariables ciVariables `json:"ciVariables"` + Project *struct { + CiVariables *ciVariables `json:"ciVariables"` } `json:"project"` } @@ -305,6 +324,16 @@ func GetGitlabProjectVariables(fullPath string, token string, instanceUrl string return variables, err } + if respData.Project == nil || respData.Project.CiVariables == nil { + // Null project/ciVariables with no transport error means the token + // cannot read the variables. Return the sentinel so the settings + // controls report not-evaluable (#418) while opportunistic callers + // (image-ref resolution) can tolerate it via errors.Is. + err := fmt.Errorf("GitLab returned a null project/ciVariables for %q (needs Maintainer+ / admin_cicd_variables): %w", fullPath, ErrProjectVariablesUnreadable) + l.WithError(err).Warn("project CI/CD variables not readable") + return variables, err + } + allNodes = append(allNodes, respData.Project.CiVariables.Nodes...) hasNextPage = respData.Project.CiVariables.PageInfo.HasNextPage cursor = respData.Project.CiVariables.PageInfo.EndCursor @@ -312,9 +341,13 @@ func GetGitlabProjectVariables(fullPath string, token string, instanceUrl string for _, v := range allNodes { newVar := CICDVariable{ - Name: v.Key, - Value: v.Value, - Type: string(v.VariableType), + Name: v.Key, + Value: v.Value, + // Normalise the GraphQL enum (ENV_VAR / FILE) to the lower-case form + // the rest of the pipeline and the REST API use ("env_var" / "file"), + // so the masked rule's file-type exclusion and the identity value stay + // consistent between real runs and fixtures. + Type: strings.ToLower(string(v.VariableType)), Protected: v.Protected, Masked: v.Masked, Hidden: v.Hidden, diff --git a/internal/ir/pipeline.go b/internal/ir/pipeline.go index 46fb91a7..5db92cac 100644 --- a/internal/ir/pipeline.go +++ b/internal/ir/pipeline.go @@ -66,6 +66,24 @@ type NormalizedPipeline struct { // (root, .github/, or docs/). Empty when the file is absent. SecurityPolicyPath string `json:"securityPolicyPath,omitempty"` + // SettingsVariables are the project's CI/CD settings variables + // (GitLab: Settings > CI/CD > Variables), each with its security + // flags — NOT the CI file's `variables:` block (that lives in + // GlobalVariables / job Variables). The value is deliberately never + // projected, per the #370 variable-sensitivity tiers, so a scan + // report never carries secret material. Empty on providers without + // settings variables. + SettingsVariables []SettingsVariable `json:"settingsVariables,omitempty"` + + // SettingsVariablesKnown is true when the settings-variable listing + // was fetched authoritatively (an empty SettingsVariables then means + // "no variables", a pass). It is false when the listing could not be + // read — a 401/403 on the settings API, or a provider that has no + // such concept — so a control keyed on these variables reports + // not-evaluable rather than a false pass. Mirrors Branch. + // ProtectionDetailsKnown. + SettingsVariablesKnown bool `json:"settingsVariablesKnown,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. @@ -400,3 +418,22 @@ type Branch struct { MinMergeAccessLevel int `json:"minMergeAccessLevel,omitempty"` ProtectionDetailsKnown bool `json:"protectionDetailsKnown,omitempty"` } + +// SettingsVariable is one project CI/CD settings variable (GitLab: Settings > +// CI/CD > Variables). It carries the variable's identity and its +// security-relevant flags only: the value is never projected, per the #370 +// variable-sensitivity tiers, so a finding can name an exposed variable +// without carrying its secret. Type is GitLab's variable_type ("env_var" or +// "file"); Environment is the environment scope ("*" for all). +type SettingsVariable struct { + Name string `json:"name"` + // Type and Environment always serialize (no omitempty): both are + // identity fields for the cicdVariablesMustBe* controls, and the rules + // read them unconditionally, so an absent key would break the finding + // or leave an inconsistent identity. GitLab always supplies both + // ("env_var"/"file" and the environment scope, "*" for all). + Type string `json:"type"` + Environment string `json:"environment"` + Protected bool `json:"protected"` + Masked bool `json:"masked"` +} diff --git a/policies/cicd_variables_must_be_masked.rego b/policies/cicd_variables_must_be_masked.rego new file mode 100644 index 00000000..61524497 --- /dev/null +++ b/policies/cicd_variables_must_be_masked.rego @@ -0,0 +1,38 @@ +# cicd-variables-must-be-masked — flag project CI/CD settings variables +# (GitLab: Settings > CI/CD > Variables, NOT the CI file's `variables:` block) +# that are not `masked`. An unmasked variable prints verbatim in every job log +# any project member can read, so a secret stored unmasked leaks to everyone +# with log access. +# +# GitLab refuses to mask a value shorter than 8 characters (or one with +# disallowed characters). Such variables are still flagged: the value is never +# projected onto the IR (per the #370 variable-sensitivity tiers), so the rule +# cannot special-case them by length, and the exposure is real regardless — +# the fix for an unmaskable secret is to restructure it, not to leave it in +# the clear. Shares its collector and identity with the protected-variable +# sibling; settingsVariablesKnown gates both so an unreadable settings API +# reports not-evaluable rather than a false pass. +# +# File-type variables (kubeconfigs, TLS keys, service-account JSON, ...) are +# excluded: GitLab does not offer the "Mask variable" option for file type, so +# it can never be masked and flagging it would be an unfixable, permanent false +# positive. Only env_var (the only maskable type) is checked. +package cicd_variables_must_be_masked + +import rego.v1 + +deny contains finding if { + input.pipeline.provider == "gitlab" + input.pipeline.settingsVariablesKnown + some v in input.pipeline.settingsVariables + lower(v.type) != "file" + not v.masked + finding := { + "code": "ISSUE-202", + "severity": "medium", + "message": sprintf("CI/CD settings variable %q is not masked — its value prints in job logs; enable masking (GitLab requires a value of at least 8 characters)", [v.name]), + "variableName": v.name, + "variableType": v.type, + "environment": v.environment, + } +} diff --git a/policies/cicd_variables_must_be_protected.rego b/policies/cicd_variables_must_be_protected.rego new file mode 100644 index 00000000..2cab8c21 --- /dev/null +++ b/policies/cicd_variables_must_be_protected.rego @@ -0,0 +1,32 @@ +# cicd-variables-must-be-protected — flag project CI/CD settings variables +# (GitLab: Settings > CI/CD > Variables, NOT the CI file's `variables:` block) +# that are not marked `protected`. An unprotected variable is injected into +# pipelines running on ANY branch, including unprotected branches any +# developer can push to, so a malicious or careless branch can exfiltrate it. +# A protected variable is only exposed to pipelines on protected branches and +# tags. +# +# The collector (gitlab/dataCollectionGitlabVariables.go) projects the +# settings variables onto input.pipeline.settingsVariables carrying identity +# and flags only — never the value (per the #370 variable-sensitivity tiers). +# input.pipeline.settingsVariablesKnown is false when the settings API could +# not be read (a 401/403 from a token without variable-read scope); the rule +# abstains then, so the control reports not-evaluable rather than a false pass. +package cicd_variables_must_be_protected + +import rego.v1 + +deny contains finding if { + input.pipeline.provider == "gitlab" + input.pipeline.settingsVariablesKnown + some v in input.pipeline.settingsVariables + not v.protected + finding := { + "code": "ISSUE-201", + "severity": "medium", + "message": sprintf("CI/CD settings variable %q is not protected — it is exposed to pipelines on unprotected branches; mark it protected so it is only injected into pipelines on protected branches and tags", [v.name]), + "variableName": v.name, + "variableType": v.type, + "environment": v.environment, + } +} diff --git a/policies/rules_test.go b/policies/rules_test.go index 4dfdcd8b..a8a2b3e3 100644 --- a/policies/rules_test.go +++ b/policies/rules_test.go @@ -1148,6 +1148,143 @@ func TestIssue501_BranchUnprotected(t *testing.T) { } } +// countCode counts findings carrying the given code. +func countCode(findings []opaengine.Finding, code string) int { + n := 0 + for _, f := range findings { + if f.Code == code { + n++ + } + } + return n +} + +// TestIssue201_CicdVariableUnprotected flags settings CI/CD variables that are +// not protected. These are settings-level findings (no file/job); identity is +// the variable's name/type/environment. The rule abstains when the settings +// listing was unreadable (SettingsVariablesKnown=false), so a token without +// variable-read scope reports not-evaluable rather than a false pass. +func TestIssue201_CicdVariableUnprotected(t *testing.T) { + engine := opaengine.New() + if err := engine.LoadFromFSFiltered(policies.FS, nil); err != nil { + t.Fatalf("load embedded policies: %v", err) + } + + // Positive: two unprotected variables, one protected (noise). + pipeline := &ir.NormalizedPipeline{ + Provider: ir.ProviderGitLab, + SettingsVariablesKnown: true, + SettingsVariables: []ir.SettingsVariable{ + {Name: "AWS_KEY", Type: "env_var", Environment: "*", Protected: false, Masked: true}, + {Name: "DEPLOY_TOKEN", Type: "env_var", Environment: "production", Protected: false, Masked: true}, + {Name: "SAFE", Type: "env_var", Environment: "*", Protected: true, Masked: true}, + }, + } + findings, err := engine.Evaluate(context.Background(), pipeline, nil) + if err != nil { + t.Fatalf("evaluate: %v", err) + } + if got := countCode(findings, "ISSUE-201"); got != 2 { + t.Fatalf("expected 2 ISSUE-201 findings, got %d", got) + } + assertSubjectKey(t, findings, "ISSUE-201", "variableName", []string{"AWS_KEY", "DEPLOY_TOKEN"}) + assertSubjectKey(t, findings, "ISSUE-201", "environment", []string{"*", "production"}) + + // Negative: everything protected -> no findings. + clean := &ir.NormalizedPipeline{ + Provider: ir.ProviderGitLab, + SettingsVariablesKnown: true, + SettingsVariables: []ir.SettingsVariable{ + {Name: "SAFE", Type: "env_var", Environment: "*", Protected: true, Masked: true}, + }, + } + if f, _ := engine.Evaluate(context.Background(), clean, nil); countCode(f, "ISSUE-201") != 0 { + t.Fatalf("expected 0 ISSUE-201 findings when all variables are protected") + } + + // Abstain: listing unreadable (Known=false) -> not-evaluable, so no + // findings even though a variable is unprotected. + unknown := &ir.NormalizedPipeline{ + Provider: ir.ProviderGitLab, + SettingsVariablesKnown: false, + SettingsVariables: []ir.SettingsVariable{ + {Name: "AWS_KEY", Type: "env_var", Environment: "*", Protected: false}, + }, + } + if f, _ := engine.Evaluate(context.Background(), unknown, nil); countCode(f, "ISSUE-201") != 0 { + t.Fatalf("expected 0 ISSUE-201 findings when the settings listing is unreadable") + } +} + +// TestIssue202_CicdVariableUnmasked flags settings CI/CD variables that are +// not masked (their values print in job logs). Same collector, identity, and +// abstain semantics as ISSUE-201. +func TestIssue202_CicdVariableUnmasked(t *testing.T) { + engine := opaengine.New() + if err := engine.LoadFromFSFiltered(policies.FS, nil); err != nil { + t.Fatalf("load embedded policies: %v", err) + } + + // Positive: one unmasked variable, one masked (noise). + pipeline := &ir.NormalizedPipeline{ + Provider: ir.ProviderGitLab, + SettingsVariablesKnown: true, + SettingsVariables: []ir.SettingsVariable{ + {Name: "PLAINTEXT_TOKEN", Type: "env_var", Environment: "*", Protected: true, Masked: false}, + {Name: "MASKED", Type: "env_var", Environment: "*", Protected: true, Masked: true}, + }, + } + findings, err := engine.Evaluate(context.Background(), pipeline, nil) + if err != nil { + t.Fatalf("evaluate: %v", err) + } + if got := countCode(findings, "ISSUE-202"); got != 1 { + t.Fatalf("expected 1 ISSUE-202 finding, got %d", got) + } + assertSubjectKey(t, findings, "ISSUE-202", "variableName", []string{"PLAINTEXT_TOKEN"}) + + // Negative: everything masked -> no findings. + clean := &ir.NormalizedPipeline{ + Provider: ir.ProviderGitLab, + SettingsVariablesKnown: true, + SettingsVariables: []ir.SettingsVariable{ + {Name: "MASKED", Type: "env_var", Environment: "*", Protected: true, Masked: true}, + }, + } + if f, _ := engine.Evaluate(context.Background(), clean, nil); countCode(f, "ISSUE-202") != 0 { + t.Fatalf("expected 0 ISSUE-202 findings when all variables are masked") + } + + // Negative: file-type variables cannot be masked in GitLab, so an unmasked + // file variable is an unfixable false positive and must NOT fire. Only the + // env_var here is flagged. Case-insensitive: the GraphQL enum form "FILE" is + // excluded the same as "file". + fileVars := &ir.NormalizedPipeline{ + Provider: ir.ProviderGitLab, + SettingsVariablesKnown: true, + SettingsVariables: []ir.SettingsVariable{ + {Name: "KUBECONFIG", Type: "file", Environment: "*", Protected: true, Masked: false}, + {Name: "TLS_KEY", Type: "FILE", Environment: "*", Protected: true, Masked: false}, + {Name: "PLAINTEXT_TOKEN", Type: "env_var", Environment: "*", Protected: true, Masked: false}, + }, + } + if f, _ := engine.Evaluate(context.Background(), fileVars, nil); countCode(f, "ISSUE-202") != 1 { + t.Fatalf("expected exactly 1 ISSUE-202 finding: file-type variables cannot be masked and must be skipped, only the env_var counts") + } + + // Abstain: listing unreadable -> not-evaluable. + unknown := &ir.NormalizedPipeline{ + Provider: ir.ProviderGitLab, + SettingsVariablesKnown: false, + SettingsVariables: []ir.SettingsVariable{ + {Name: "PLAINTEXT_TOKEN", Type: "env_var", Environment: "*", Masked: false}, + }, + } + if f, _ := engine.Evaluate(context.Background(), unknown, nil); countCode(f, "ISSUE-202") != 0 { + t.Fatalf("expected 0 ISSUE-202 findings when the settings listing is unreadable") + } +} + // TestIssue505_BranchNonCompliant flags protected branches whose // settings fail to meet the declared minimum bar. func TestIssue505_BranchNonCompliant(t *testing.T) {