From 18c8fc82c95a77970f248cee5b1f2d8a036c180f Mon Sep 17 00:00:00 2001 From: k3nny Date: Thu, 11 Jun 2026 23:13:25 +0200 Subject: [PATCH] feat(linter): add GL032 variable reference validation in rules:if: Warn when a $VAR or ${VAR} reference in a rules:if: expression is not declared in pipeline variables:, the job's own variables:, or any workflow:rules:variables: block. Predefined GitLab CI namespaces (CI_*, GITLAB_*, FF_*, RUNNER_*, TRIGGER_*, CHAT_*) are always exempt. Each undeclared variable is reported at most once per job. The finding is a WARNING (not an error) because variables may also be set in GitLab CI/CD project settings, which are invisible to glint at lint time. Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.md | 2 + README.md | 6 + Taskfile.yml | 2 + internal/linter/linter.go | 1 + internal/linter/rules.go | 7 + internal/linter/variables.go | 158 ++++++++++++++++++++++ internal/linter/variables_test.go | 214 ++++++++++++++++++++++++++++++ testdata/variable_refs.yml | 51 +++++++ 8 files changed, 441 insertions(+) create mode 100644 internal/linter/variables.go create mode 100644 internal/linter/variables_test.go create mode 100644 testdata/variable_refs.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index 7af0ba5..d2f2fc5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ This project uses [Semantic Versioning](https://semver.org). ### Added +- **Variable reference validation (GL032)** — glint now warns when a `rules:if:` expression references a variable (`$VAR` or `${VAR}`) that is not declared anywhere in the pipeline YAML: pipeline-level `variables:`, the job's own `variables:`, or any `workflow:rules:variables:` block. Predefined GitLab CI variable namespaces (`CI_*`, `GITLAB_*`, `FF_*`, `RUNNER_*`, `TRIGGER_*`, `CHAT_*`) are exempt. Because variables can also be set in GitLab CI/CD project settings (invisible to glint), the finding is a `[WARNING]` rather than an error. Each undeclared variable is reported at most once per job to keep the output concise. + - **Structured rule IDs** — every finding now carries a stable `GL###` identifier (e.g. `GL003`) that appears in the output between the location and the message: `[ERROR] job "deploy" (file.yml:14) GL003: missing required field 'script'`. IDs are assigned per check function across 31 rules (GL001–GL031) and are stable across versions. The `linter.Finding` struct exposes the ID as a `Rule string` field for programmatic consumers. The README lint rules table is updated with ID columns. - **`workflow:rules:variables:` now propagate to job rule evaluation** — when a `workflow:rules:` entry matches, any `variables:` it defines are injected into the evaluation context so job `rules:if:` expressions can reference them. Pipeline-level `variables:` are also available as defaults (lower priority). Variable priority order, highest first: `--var` CLI overrides → `--branch`/`--tag`/`--source` shortcuts → workflow-rule variables → pipeline-level variable defaults. This means `$DEPLOY_TARGET == "production"` in a job rule correctly evaluates when a workflow rule sets `DEPLOY_TARGET: production` for the matching branch. The `glint graph tree` command benefits from the same enrichment. diff --git a/README.md b/README.md index 5696e87..5501faf 100644 --- a/README.md +++ b/README.md @@ -298,6 +298,12 @@ Every finding includes a stable rule ID (e.g. `GL003`) that can be used to filte | GL030 | ERROR | `dependencies:` references a job that does not exist | | GL031 | ERROR | `dependencies:` references a job in the same or a later stage | +### Expression validation + +| ID | Severity | Rule | +|----|----------|------| +| GL032 | WARNING | `rules:if:` references `$VAR` not declared in `variables:` (pipeline, job, or `workflow:rules:variables:`) — may be a false positive for variables set in GitLab CI/CD project settings | + ### Hidden jobs (templates) Jobs whose name starts with `.` are treated as reusable templates and skipped for most rules. This matches GitLab's own behaviour. diff --git a/Taskfile.yml b/Taskfile.yml index acf2697..66975e4 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -73,6 +73,8 @@ tasks: ignore_error: false - cmd: ./{{.BINARY}} check --branch feat/x testdata/workflow_vars.yml ignore_error: false + - cmd: ./{{.BINARY}} check testdata/variable_refs.yml + ignore_error: false - cmd: ./{{.BINARY}} check samba-testdata/.gitlab-ci.yml ignore_error: false - cmd: ./{{.BINARY}} check samba-testdata/.gitlab-ci-coverage.yml diff --git a/internal/linter/linter.go b/internal/linter/linter.go index 203dd84..c9496e6 100644 --- a/internal/linter/linter.go +++ b/internal/linter/linter.go @@ -53,6 +53,7 @@ func Lint(p *model.Pipeline) []Finding { findings = append(findings, checkJobs(p)...) findings = append(findings, checkNeeds(p)...) findings = append(findings, checkDependencies(p)...) + findings = append(findings, checkVariableRefs(p)...) return findings } diff --git a/internal/linter/rules.go b/internal/linter/rules.go index 5ab2129..4a82245 100644 --- a/internal/linter/rules.go +++ b/internal/linter/rules.go @@ -107,4 +107,11 @@ const ( // GL031: dependencies: references a job in the same or a later stage. RuleDependencyStage = "GL031" + + // ── Expression validation ──────────────────────────────────────────────── + + // GL032: rules:if: references a variable not declared in pipeline variables:, + // the job's own variables:, or any workflow:rules:variables: block. + // May be a false positive for variables set in GitLab CI/CD project settings. + RuleUndeclaredVariable = "GL032" ) diff --git a/internal/linter/variables.go b/internal/linter/variables.go new file mode 100644 index 0000000..d89bac6 --- /dev/null +++ b/internal/linter/variables.go @@ -0,0 +1,158 @@ +package linter + +import ( + "fmt" + "strings" + + "git.k3nny.fr/glint/internal/model" +) + +// predefinedVarPrefixes lists GitLab-maintained variable namespaces that are +// always available without an explicit declaration in variables: blocks. +var predefinedVarPrefixes = []string{ + "CI_", // most predefined CI variables (CI_COMMIT_BRANCH, CI_JOB_ID, …) + "GITLAB_", // user/project metadata (GITLAB_USER_ID, GITLAB_FEATURES, …) + "FF_", // GitLab feature flags + "RUNNER_", // runner-level variables + "TRIGGER_", // trigger token variables passed from upstream pipelines + "CHAT_", // ChatOps variables +} + +func isPredefinedVar(name string) bool { + for _, prefix := range predefinedVarPrefixes { + if strings.HasPrefix(name, prefix) { + return true + } + } + return false +} + +// extractIfVars returns every variable name referenced in a rules:if: expression. +// String literals are skipped so that dollar signs inside quoted values are +// not mistaken for variable references. +func extractIfVars(expr string) []string { + var names []string + i := 0 + for i < len(expr) { + switch expr[i] { + case '"', '\'': + quote := expr[i] + i++ + for i < len(expr) && expr[i] != quote { + if expr[i] == '\\' { + i++ // skip escaped character + } + i++ + } + if i < len(expr) { + i++ // consume closing quote + } + case '$': + i++ // consume '$' + if i < len(expr) && expr[i] == '{' { + i++ // consume '{' + start := i + for i < len(expr) && isVarNameByte(expr[i]) { + i++ + } + if i < len(expr) && expr[i] == '}' && i > start { + names = append(names, expr[start:i]) + i++ // consume '}' + } + } else { + start := i + for i < len(expr) && isVarNameByte(expr[i]) { + i++ + } + if i > start { + names = append(names, expr[start:i]) + } + } + default: + i++ + } + } + return names +} + +func isVarNameByte(b byte) bool { + return (b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') || (b >= '0' && b <= '9') || b == '_' +} + +// checkVariableRefs warns when a rules:if: expression references a variable that +// is not declared in pipeline variables:, the job's own variables:, or any +// workflow:rules:variables: block. Predefined GitLab CI variables (CI_*, GITLAB_*, +// …) are always exempt. Variables set in GitLab CI/CD project settings are +// invisible to glint, so the finding is a WARNING rather than an error. +func checkVariableRefs(p *model.Pipeline) []Finding { + pipelineVars := make(map[string]bool, len(p.Variables)) + for k := range p.Variables { + pipelineVars[k] = true + } + + // Union of all variables any workflow rule might inject into the context. + workflowRuleVars := make(map[string]bool) + if p.Workflow != nil { + for _, rule := range p.Workflow.Rules { + for k := range rule.Variables { + workflowRuleVars[k] = true + } + } + } + + var findings []Finding + + // Check workflow rules:if: expressions. + if p.Workflow != nil { + seen := make(map[string]bool) + for i, rule := range p.Workflow.Rules { + if rule.If == "" { + continue + } + for _, varName := range extractIfVars(rule.If) { + if isPredefinedVar(varName) || pipelineVars[varName] || seen[varName] { + continue + } + seen[varName] = true + findings = append(findings, Finding{ + Severity: Warning, + Rule: RuleUndeclaredVariable, + File: p.SourceFile, + Message: fmt.Sprintf("workflow.rules[%d].if: $%s is not declared in pipeline variables:", i, varName), + }) + } + } + } + + // Check each job's rules:if: expressions. + for name, job := range p.Jobs { + jobVars := make(map[string]bool, len(job.Variables)) + for k := range job.Variables { + jobVars[k] = true + } + + // Deduplicate per (job, varName): report each undeclared variable once per job. + seen := make(map[string]bool) + for i, rule := range job.Rules { + if rule.If == "" { + continue + } + for _, varName := range extractIfVars(rule.If) { + if isPredefinedVar(varName) || pipelineVars[varName] || jobVars[varName] || workflowRuleVars[varName] || seen[varName] { + continue + } + seen[varName] = true + findings = append(findings, Finding{ + Severity: Warning, + Rule: RuleUndeclaredVariable, + Job: name, + File: job.File, + Line: job.Line, + Message: fmt.Sprintf("rules[%d].if: $%s is not declared in pipeline or job variables:", i, varName), + }) + } + } + } + + return findings +} diff --git a/internal/linter/variables_test.go b/internal/linter/variables_test.go new file mode 100644 index 0000000..774c4c8 --- /dev/null +++ b/internal/linter/variables_test.go @@ -0,0 +1,214 @@ +package linter + +import ( + "testing" + + "git.k3nny.fr/glint/internal/model" +) + +func TestExtractIfVars(t *testing.T) { + cases := []struct { + name string + expr string + want []string + }{ + { + name: "simple variable", + expr: `$MY_VAR == "value"`, + want: []string{"MY_VAR"}, + }, + { + name: "curly brace syntax", + expr: `${MY_VAR} != null`, + want: []string{"MY_VAR"}, + }, + { + name: "multiple variables", + expr: `$BRANCH == "main" && $DEPLOY_ENV == "prod"`, + want: []string{"BRANCH", "DEPLOY_ENV"}, + }, + { + name: "dollar sign inside string literal is skipped", + expr: `$REAL_VAR == "$not_a_var"`, + want: []string{"REAL_VAR"}, + }, + { + name: "no variables", + expr: `"main" == "main"`, + want: nil, + }, + { + name: "regex rhs variable", + expr: `$BRANCH =~ $PATTERN`, + want: []string{"BRANCH", "PATTERN"}, + }, + { + name: "multiline expression", + expr: "$BRANCH == \"main\" ||\n$BRANCH == \"develop\"", + want: []string{"BRANCH", "BRANCH"}, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := extractIfVars(tc.expr) + if len(got) != len(tc.want) { + t.Fatalf("extractIfVars(%q) = %v, want %v", tc.expr, got, tc.want) + } + for i, v := range got { + if v != tc.want[i] { + t.Errorf("[%d] got %q, want %q", i, v, tc.want[i]) + } + } + }) + } +} + +func TestIsPredefinedVar(t *testing.T) { + cases := []struct { + input string + want bool + }{ + {"CI_COMMIT_BRANCH", true}, + {"CI_JOB_TOKEN", true}, + {"GITLAB_USER_ID", true}, + {"GITLAB_FEATURES", true}, + {"FF_SOME_FLAG", true}, + {"RUNNER_ID", true}, + {"TRIGGER_PAYLOAD", true}, + {"CHAT_INPUT", true}, + {"MY_CUSTOM_VAR", false}, + {"DEPLOY_ENV", false}, + {"FEATURE_ENABLED", false}, + } + for _, tc := range cases { + t.Run(tc.input, func(t *testing.T) { + if got := isPredefinedVar(tc.input); got != tc.want { + t.Errorf("isPredefinedVar(%q) = %v, want %v", tc.input, got, tc.want) + } + }) + } +} + +func TestCheckVariableRefs(t *testing.T) { + cases := []struct { + name string + pipeline *model.Pipeline + wantWarnings int + }{ + { + name: "declared pipeline variable — no warning", + pipeline: &model.Pipeline{ + Variables: map[string]any{"MY_VAR": "value"}, + Jobs: map[string]model.Job{ + "job-a": {Rules: []model.Rule{{If: `$MY_VAR == "value"`}}}, + }, + }, + wantWarnings: 0, + }, + { + name: "predefined CI variable — no warning", + pipeline: &model.Pipeline{ + Jobs: map[string]model.Job{ + "job-a": {Rules: []model.Rule{{If: `$CI_COMMIT_BRANCH == "main"`}}}, + }, + }, + wantWarnings: 0, + }, + { + name: "undeclared variable — one warning", + pipeline: &model.Pipeline{ + Jobs: map[string]model.Job{ + "job-a": {Rules: []model.Rule{{If: `$UNDEFINED_VAR == "yes"`}}}, + }, + }, + wantWarnings: 1, + }, + { + name: "same undeclared var in multiple rules — one warning per job", + pipeline: &model.Pipeline{ + Jobs: map[string]model.Job{ + "job-a": { + Rules: []model.Rule{ + {If: `$UNDEFINED_VAR == "yes"`}, + {If: `$UNDEFINED_VAR == "no"`}, + }, + }, + }, + }, + wantWarnings: 1, + }, + { + name: "job-level variable — no warning", + pipeline: &model.Pipeline{ + Jobs: map[string]model.Job{ + "job-a": { + Variables: map[string]any{"LOCAL_VAR": "value"}, + Rules: []model.Rule{{If: `$LOCAL_VAR == "value"`}}, + }, + }, + }, + wantWarnings: 0, + }, + { + name: "workflow rule variable available to job rules — no warning", + pipeline: &model.Pipeline{ + Workflow: &model.Workflow{ + Rules: []model.Rule{ + { + If: `$CI_COMMIT_BRANCH == "main"`, + Variables: map[string]any{"DEPLOY_ENV": "production"}, + }, + }, + }, + Jobs: map[string]model.Job{ + "job-a": {Rules: []model.Rule{{If: `$DEPLOY_ENV == "production"`}}}, + }, + }, + wantWarnings: 0, + }, + { + name: "undeclared variable in workflow rules:if", + pipeline: &model.Pipeline{ + Workflow: &model.Workflow{ + Rules: []model.Rule{{If: `$UNDECLARED == "main"`}}, + }, + Jobs: map[string]model.Job{}, + }, + wantWarnings: 1, + }, + { + name: "two jobs each with a different undeclared variable — two warnings", + pipeline: &model.Pipeline{ + Jobs: map[string]model.Job{ + "job-a": {Rules: []model.Rule{{If: `$UNDEF_A == "x"`}}}, + "job-b": {Rules: []model.Rule{{If: `$UNDEF_B == "y"`}}}, + }, + }, + wantWarnings: 2, + }, + { + name: "no rules — no warnings", + pipeline: &model.Pipeline{ + Jobs: map[string]model.Job{ + "job-a": {Script: "echo ok"}, + }, + }, + wantWarnings: 0, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + findings := checkVariableRefs(tc.pipeline) + count := 0 + for _, f := range findings { + if f.Severity == Warning && f.Rule == RuleUndeclaredVariable { + count++ + } + } + if count != tc.wantWarnings { + t.Errorf("got %d GL032 warnings, want %d; findings: %v", count, tc.wantWarnings, findings) + } + }) + } +} diff --git a/testdata/variable_refs.yml b/testdata/variable_refs.yml new file mode 100644 index 0000000..55de4cf --- /dev/null +++ b/testdata/variable_refs.yml @@ -0,0 +1,51 @@ +--- +# variable_refs.yml +# Verifies that GL032 does not fire for variables that are declared or predefined. +# Covers: pipeline variables:, job variables:, workflow:rules:variables:, and CI_* prefixes. +# Expected: exits 0 (no errors; no GL032 warnings). + +stages: + - build + - deploy + +variables: + DEPLOY_TARGET: "staging" + FEATURE_FLAG: "false" + +workflow: + rules: + - if: '$CI_COMMIT_BRANCH == "main"' + variables: + DEPLOY_TARGET: "production" + - when: always + +build: + stage: build + script: make build + rules: + # pipeline-level variable — GL032 must not fire + - if: '$DEPLOY_TARGET == "staging"' + when: on_success + # predefined CI variable — GL032 must not fire + - if: '$CI_COMMIT_BRANCH =~ /^feat\/.+/' + when: manual + +deploy: + stage: deploy + script: make deploy + variables: + DEPLOY_REGION: "us-east-1" + rules: + # pipeline-level variable — no warning + - if: '$FEATURE_FLAG == "true"' + when: never + # workflow-rule-injected variable — no warning + - if: '$DEPLOY_TARGET == "production"' + when: on_success + # job-level variable — no warning + - if: '$DEPLOY_REGION == "us-east-1"' + when: on_success + # predefined GITLAB_ variable — no warning + - if: '$GITLAB_USER_LOGIN == "bot"' + when: never + - when: manual