Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .plumber.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
33 changes: 32 additions & 1 deletion cmd/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)"
Expand Down Expand Up @@ -146,6 +146,8 @@ type initWizardState struct {

DebugTraceEnabled bool
UnsafeExpansionEnabled bool
VarsProtectedEnabled bool
VarsMaskedEnabled bool

RequireComponents bool
RequiredComponentsExpr string
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -1045,6 +1070,12 @@ func (st *initWizardState) applyVariableControls(gl *configuration.ProviderConfi
AllowedPatterns: parseLinesInit(st.AllowedPatternsMultiline),
}
}
if st.VarsProtectedEnabled {
Comment thread
Joseph94m marked this conversation as resolved.
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 {
Expand Down
42 changes: 42 additions & 0 deletions cmd/init_variables_test.go
Original file line number Diff line number Diff line change
@@ -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")
}
})
}
49 changes: 49 additions & 0 deletions cmd/legacy_json.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
},
Comment thread
Joseph94m marked this conversation as resolved.
"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 {
Expand Down
68 changes: 68 additions & 0 deletions cmd/legacy_json_variables_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
32 changes: 32 additions & 0 deletions cmd/render_details.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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":
Comment thread
Joseph94m marked this conversation as resolved.
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
Expand Down
32 changes: 32 additions & 0 deletions configuration/plumberconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ var validControlSchema = map[string][]string{
"allowForcePush", "codeOwnerApprovalRequired",
"minMergeAccessLevel", "minPushAccessLevel",
},
"cicdVariablesMustBeProtected": {
"enabled",
},
"cicdVariablesMustBeMasked": {
"enabled",
},
"pipelineMustNotIncludeHardcodedJobs": {
"enabled",
},
Expand Down Expand Up @@ -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"`

Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions configuration/plumberconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,8 @@ func TestValidControlNames(t *testing.T) {
"actionsMustNotCarryKnownCVEs",
"actionsMustNotExecuteMutableRemoteCode",
"branchMustBeProtected",
"cicdVariablesMustBeMasked",
"cicdVariablesMustBeProtected",
"containerImageMustComeFromAuthorizedSources",
"containerImageMustNotUseForbiddenTags",
"externalRefsMustNotCollide",
Expand Down
2 changes: 2 additions & 0 deletions configuration/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}},
Expand Down
Loading
Loading