From 9af0b29fd195b6a272219d0a6644519a1b74773e Mon Sep 17 00:00:00 2001 From: Pavel Okhlopkov Date: Wed, 19 Aug 2026 17:33:39 +0300 Subject: [PATCH 1/8] add linter Signed-off-by: Pavel Okhlopkov --- internal/modules/module.go | 2 + pkg/config.go | 2 + pkg/config/global/global.go | 1 + pkg/config/linters_settings.go | 2 + pkg/linters/templates/README.md | 93 +++ .../templates/rules/openapi_values_quote.go | 639 ++++++++++++++++++ .../rules/openapi_values_quote_test.go | 433 ++++++++++++ pkg/linters/templates/templates.go | 4 + .../openapi-values-quote/expected.yaml | 15 + .../openapi-values-quote/module/module.yaml | 2 + .../module/openapi/config-values.yaml | 9 + .../module/openapi/values.yaml | 4 + .../module/templates/configmap.yaml | 10 + 13 files changed, 1216 insertions(+) create mode 100644 pkg/linters/templates/rules/openapi_values_quote.go create mode 100644 pkg/linters/templates/rules/openapi_values_quote_test.go create mode 100644 test/e2e/testdata/templates/openapi-values-quote/expected.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote/module/module.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote/module/openapi/config-values.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote/module/openapi/values.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote/module/templates/configmap.yaml diff --git a/internal/modules/module.go b/internal/modules/module.go index 96762d94..ec1b3cd3 100644 --- a/internal/modules/module.go +++ b/internal/modules/module.go @@ -367,6 +367,7 @@ func mapTemplatesRules(linterSettings *pkg.LintersSettings, configSettings *conf rules.MountPointsRule.SetLevel(globalRules.MountPointsRule.Impact, fallbackImpact) rules.WebhookConfigurationRule.SetLevel(globalRules.WebhookConfigurationRule.Impact, fallbackImpact) rules.HelmRenderRule.SetLevel(globalRules.HelmRenderRule.Impact, fallbackImpact) + rules.OpenAPIValuesQuoteRule.SetLevel(globalRules.OpenAPIValuesQuoteRule.Impact, fallbackImpact) } // mapOpenAPIRules configures OpenAPI linter rules @@ -486,6 +487,7 @@ func mapTemplatesExclusionsAndSettings(linterSettings *pkg.LintersSettings, conf excludes.EnabledModules.Directories = pkg.DirectoryRuleExcludeList(configExcludes.EnabledModules.Directories) excludes.WebhookConfiguration = configExcludes.WebhookConfiguration.Get() excludes.MountPoints = pkg.StringRuleExcludeList(configExcludes.MountPoints) + excludes.OpenAPIValuesQuote = pkg.StringRuleExcludeList(configExcludes.OpenAPIValuesQuote) // Additional settings linterSettings.Templates.PrometheusRuleSettings.Disable = configSettings.Templates.PrometheusRules.Disable diff --git a/pkg/config.go b/pkg/config.go index a318bf8c..7d78dd8c 100644 --- a/pkg/config.go +++ b/pkg/config.go @@ -147,6 +147,7 @@ type TemplatesLinterRules struct { WebhookConfigurationRule RuleConfig MountPointsRule RuleConfig HelmRenderRule RuleConfig + OpenAPIValuesQuoteRule RuleConfig } type PrometheusRuleSettings struct { @@ -166,6 +167,7 @@ type TemplatesExcludeRules struct { EnabledModules EnabledModulesExcludeRule WebhookConfiguration KindRuleExcludeList MountPoints StringRuleExcludeList + OpenAPIValuesQuote StringRuleExcludeList } type EnabledModulesExcludeRule struct { diff --git a/pkg/config/global/global.go b/pkg/config/global/global.go index 845f3891..a1093e0c 100644 --- a/pkg/config/global/global.go +++ b/pkg/config/global/global.go @@ -147,6 +147,7 @@ type TemplatesLinterRules struct { WebhookConfigurationRule RuleConfig `mapstructure:"webhook-configuration-annotations"` MountPointsRule RuleConfig `mapstructure:"mount-points"` HelmRenderRule RuleConfig `mapstructure:"helm-render"` + OpenAPIValuesQuoteRule RuleConfig `mapstructure:"openapi-values-quote"` } func (c LinterConfig) IsWarn() bool { diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 28cb1c04..d28d6717 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -235,6 +235,7 @@ type TemplatesLinterRules struct { WebhookConfigurationRule RuleConfig `mapstructure:"webhook-configuration-annotations"` MountPointsRule RuleConfig `mapstructure:"mount-points"` HelmRenderRule RuleConfig `mapstructure:"helm-render"` + OpenAPIValuesQuoteRule RuleConfig `mapstructure:"openapi-values-quote"` } type TemplatesExcludeRules struct { @@ -247,6 +248,7 @@ type TemplatesExcludeRules struct { EnabledModules EnabledModulesExcludeRule `mapstructure:"enabled-modules"` WebhookConfiguration KindRuleExcludeList `mapstructure:"webhook-configuration-annotations"` MountPoints StringRuleExcludeList `mapstructure:"mount-points"` + OpenAPIValuesQuote StringRuleExcludeList `mapstructure:"openapi-values-quote"` } type EnabledModulesExcludeRule struct { diff --git a/pkg/linters/templates/README.md b/pkg/linters/templates/README.md index 8c95d4b3..8e0ff4dd 100644 --- a/pkg/linters/templates/README.md +++ b/pkg/linters/templates/README.md @@ -24,6 +24,7 @@ Proper template validation prevents runtime issues, ensures applications are pro | [enabled-modules](#enabled-modules) | Detects usage of `.Values.global.enabledModules` in templates | ✅ | enabled | | [webhook-configuration-annotations](#webhook-configuration-annotations) | Checks webhook configurations have werf.io/weight or deploy-dependency annotations | ✅ | enabled | | [mount-points](#mount-points) | Validates that mount-points.yaml directories are used as volumeMounts in pod controllers | ✅ | enabled | +| [openapi-values-quote](#openapi-values-quote) | Requires templates to quote OpenAPI string values that have no validation pattern | ✅ | enabled | "Configurable" means that this rule can be configured using the `.dmtlint.yaml` file, including customizing the rule's parameters and/or disabling the rule. @@ -2036,6 +2037,91 @@ linters-settings: **Configuration:** +### openapi-values-quote + +**Purpose:** Ensures that module OpenAPI string values without a validation pattern are always quoted when rendered into templates. An unconstrained string can contain characters that break YAML or silently change the parsed type (for example `123`, `true`, `on`, a value with a leading `0`, or one containing `:` or `#`), so it must be quoted at the point of use. + +**Description:** + +The rule reads the module value schema (`openapi/values.yaml` and `openapi/config-values.yaml`), collects every value path whose type resolves to a **string with no `pattern`, `enum`, or `format`**, and then checks the module `templates/` for usages of those values (`.Values..`). A usage that is rendered as a bare YAML value — not piped through `quote`/`squote` (or another YAML-safe function such as `b64enc`, `toJson`, `sha256sum`), and not wrapped in quotes — is reported. + +**What counts as a string:** + +- a direct `type: string` property; +- a nullable string (`type: ["string", "null"]`); +- a string reached through `$ref`, `allOf`, `oneOf`, or `anyOf`; +- items of a string array (`type: array` with `items: {type: string}`), checked through `range`. + +**What makes a string safe (not reported):** + +- it declares a `pattern`, an `enum`, or a `format` in the schema; **or** +- every template usage quotes it (`| quote`, `| squote`, or literal `"…"`/`'…'` wrapping), or pipes it through a function whose output is always YAML-safe. + +To keep findings high-confidence, the rule only reports **standalone** values (`key: {{ … }}` or `- {{ … }}`). A value embedded in a larger scalar (`host: prefix-{{ … }}`), used in a condition (`{{ if … }}`), or placed inside a YAML block scalar (`|` / `>`) is not reported. + +**Why it matters:** + +Rendering an unconstrained string unquoted is a classic source of broken manifests and subtle type bugs: a value like `123456` becomes an integer, `true`/`no` becomes a boolean, and a value containing `:` or a leading `@` can make the document fail to parse. A schema `pattern`/`enum`/`format` already guarantees the value is safe; without one, the template must quote it. + +**Examples:** + +Given the module schema: + +```yaml +# openapi/config-values.yaml +type: object +properties: + greeting: # no pattern -> must be quoted in templates + type: string + code: # constrained -> quoting not required + type: string + pattern: '^[a-z]+$' +``` + +❌ **Incorrect** — pattern-less string rendered unquoted: + +```yaml +# templates/configmap.yaml +data: + greeting: {{ .Values.myModule.greeting }} +``` + +✅ **Correct** — quote the value: + +```yaml +# templates/configmap.yaml +data: + greeting: {{ .Values.myModule.greeting | quote }} + # or: greeting: "{{ .Values.myModule.greeting }}" +``` + +For string arrays, quote each element inside the loop: + +```yaml +args: +{{- range .Values.myModule.extraArgs }} + - {{ . | quote }} +{{- end }} +``` + +**Configuration:** + +Set the impact level or exclude specific value paths (matched against the dotted path relative to the module values root, e.g. `foo.bar`): + +```yaml +# .dmtlint.yaml +linters-settings: + templates: + rules: + openapi-values-quote: + impact: error + exclude-rules: + openapi-values-quote: + - internal.someLegacyField + - extraArgs +``` + + ## Configuration The Templates linter can be configured at the module level with rule-specific settings and exclusions. @@ -2092,6 +2178,8 @@ linters-settings: impact: warning webhook-configuration-annotations: impact: error + openapi-values-quote: + impact: error ``` ### Rule-Level Exclusions @@ -2156,6 +2244,11 @@ linters-settings: name: istio-sidecar-injector - kind: MutatingWebhookConfiguration name: cert-manager-webhook + + # openapi-values-quote exclusions (by value path, relative to the module values root) + openapi-values-quote: + - internal.someLegacyField + - extraArgs ``` ### Complete Configuration Example diff --git a/pkg/linters/templates/rules/openapi_values_quote.go b/pkg/linters/templates/rules/openapi_values_quote.go new file mode 100644 index 00000000..f9091a1b --- /dev/null +++ b/pkg/linters/templates/rules/openapi_values_quote.go @@ -0,0 +1,639 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rules + +import ( + "maps" + "os" + "path/filepath" + "regexp" + "strings" + + "sigs.k8s.io/yaml" + + "github.com/deckhouse/dmt/internal/fsutils" + "github.com/deckhouse/dmt/internal/modules/values" + "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/errors" +) + +// OpenAPIValuesQuoteRuleName is the rule id reported in findings and used as the +// config key (templates.rules.openapi-values-quote / exclude-rules key). +const OpenAPIValuesQuoteRuleName = "openapi-values-quote" + +// openAPIValuesFiles are the module value-schema files that define the +// .Values. subtree consumed by templates. +var openAPIValuesFiles = []string{"values.yaml", "config-values.yaml"} + +// safeFuncs are Helm/sprig pipeline functions whose output is guaranteed to be a +// YAML-safe scalar — either already quoted (quote/squote/toJson) or restricted to +// an alphabet that never needs quoting (base-N encoders, checksums). A value piped +// through one of them does not need an explicit quote. +var safeFuncs = map[string]struct{}{ + "quote": {}, "squote": {}, + "toJson": {}, "mustToJson": {}, "toRawJson": {}, "mustToRawJson": {}, + "b64enc": {}, "b32enc": {}, + "sha1sum": {}, "sha256sum": {}, "sha512sum": {}, "adler32sum": {}, +} + +// templateActionRe matches a single Go-template action {{ ... }}, capturing the +// inner expression with the surrounding whitespace/`-` trim markers stripped. +var templateActionRe = regexp.MustCompile(`{{-?\s*(.*?)\s*-?}}`) + +// blockScalarHeaderRe matches a YAML block-scalar header line (`key: |`, `- >-`, +// `key: |+2 # x`, …). Content indented under such a header is a literal string, so +// substitutions there are already part of a string and must not be quoted. +var blockScalarHeaderRe = regexp.MustCompile(`^(\s*)(-\s+)?([\w.\-/]+\s*:\s*)?[|>][+-]?\d*\s*(#.*)?$`) + +// standaloneOpenRe matches the text before an action when the action is the whole +// YAML value: indentation, an optional list dash, an optional `key:`, and an +// optional opening quote (captured). +var standaloneOpenRe = regexp.MustCompile(`^\s*(-\s+)?([\w.\-/]+\s*:\s*)?(["']?)\s*$`) + +// OpenAPIValuesQuoteRule flags template usages of module OpenAPI string values that +// have no validation pattern (no pattern/enum/format) and are rendered unquoted. +// Such values can contain characters that break YAML or silently change the parsed +// type, so they must be quoted in templates. +type OpenAPIValuesQuoteRule struct { + pkg.RuleMeta + pkg.StringRule +} + +func NewOpenAPIValuesQuoteRule(excludeRules []pkg.StringRuleExclude) *OpenAPIValuesQuoteRule { + return &OpenAPIValuesQuoteRule{ + RuleMeta: pkg.RuleMeta{ + Name: OpenAPIValuesQuoteRuleName, + }, + StringRule: pkg.StringRule{ + ExcludeRules: excludeRules, + }, + } +} + +// riskyPaths holds the value paths (relative to the module values root) whose type +// resolves to an unvalidated string. scalar paths are string leaves; array paths are +// arrays whose items are unvalidated strings (checked through `range`). +type riskyPaths struct { + scalar map[string]struct{} + array map[string]struct{} +} + +func newRiskyPaths() *riskyPaths { + return &riskyPaths{ + scalar: make(map[string]struct{}), + array: make(map[string]struct{}), + } +} + +func (p *riskyPaths) empty() bool { + return len(p.scalar) == 0 && len(p.array) == 0 +} + +// CheckStringValuesQuoted parses the module OpenAPI value schema, collects the +// pattern-less string paths, and reports every unquoted usage of them in templates. +func (r *OpenAPIValuesQuoteRule) CheckStringValuesQuoted(m pkg.Module, errorList *errors.LintRuleErrorsList) { + errorList = errorList.WithRule(r.GetName()) + + modulePath := m.GetPath() + valuesKey := values.ModuleCamelName(m.GetName()) + + risky := newRiskyPaths() + + for _, name := range openAPIValuesFiles { + data, err := os.ReadFile(filepath.Join(modulePath, "openapi", name)) + if err != nil { + continue + } + + schema := make(map[string]any) + if err := yaml.Unmarshal(data, &schema); err != nil { + // A malformed schema is reported by the openapi linter; skip it here. + continue + } + + (&schemaWalker{root: schema, out: risky}).walk("", schema, 0) + } + + if risky.empty() { + return + } + + templatesPath := filepath.Join(modulePath, "templates") + if _, err := os.Stat(templatesPath); os.IsNotExist(err) { + return + } + + files := fsutils.GetFiles(templatesPath, true, + fsutils.FilterFileByExtensions(".yaml", ".yml", ".tpl", ".tpl.yaml", ".tpl.yml")) + + for _, filePath := range files { + content, err := os.ReadFile(filePath) + if err != nil { + continue + } + + relPath := fsutils.Rel(modulePath, filePath) + lines := strings.Split(string(content), "\n") + inBlock := computeBlockScalarLines(lines) + + r.checkScalars(relPath, lines, inBlock, valuesKey, risky, errorList) + r.checkArrayRanges(relPath, lines, inBlock, valuesKey, risky, errorList) + } +} + +// checkScalars reports unquoted standalone emissions of scalar string values. +func (r *OpenAPIValuesQuoteRule) checkScalars( + relPath string, lines []string, inBlock []bool, valuesKey string, + risky *riskyPaths, errorList *errors.LintRuleErrorsList, +) { + prefix := ".Values." + valuesKey + "." + + for i, line := range lines { + if inBlock[i] { + continue + } + + for _, loc := range templateActionRe.FindAllStringSubmatchIndex(line, -1) { + inner := strings.TrimSpace(line[loc[2]:loc[3]]) + + cmd := leadingCommand(inner) + if !strings.HasPrefix(cmd, prefix) { + continue + } + + valPath := cmd[len(prefix):] + if _, ok := risky.scalar[valPath]; !ok { + continue + } + + if !r.Enabled(valPath) { + continue + } + + if pipelineIsSafe(inner) { + continue + } + + if standalone, quoted := analyzeContext(line[:loc[0]], line[loc[1]:]); !standalone || quoted { + continue + } + + errorList.WithFilePath(relPath).WithLineNumber(i+1). + Errorf("value '.Values.%s.%s' is an OpenAPI string without a validation pattern (pattern/enum/format) and must be quoted in templates: use '| quote' or wrap the value in quotes", + valuesKey, valPath) + } + } +} + +// arrayScope tracks a template block while scanning for `range` loops over risky +// string arrays. elemVar is the loop element variable ("$x" or "." for dot-binding) +// when the range iterates a risky array; it is empty for any other block. +type arrayScope struct { + rebindDot bool + elemVar string + arrPath string +} + +// checkArrayRanges reports unquoted emissions of elements while ranging over a risky +// string array (`{{- range $e := .Values.mod.list }}{{ $e }}{{- end }}`). +func (r *OpenAPIValuesQuoteRule) checkArrayRanges( + relPath string, lines []string, inBlock []bool, valuesKey string, + risky *riskyPaths, errorList *errors.LintRuleErrorsList, +) { + if len(risky.array) == 0 { + return + } + + prefix := ".Values." + valuesKey + "." + + var stack []arrayScope + + innermostDotScope := func() *arrayScope { + for i := len(stack) - 1; i >= 0; i-- { + if stack[i].rebindDot { + return &stack[i] + } + } + + return nil + } + + findVarScope := func(v string) *arrayScope { + for i := len(stack) - 1; i >= 0; i-- { + if stack[i].elemVar == v { + return &stack[i] + } + } + + return nil + } + + for i, line := range lines { + for _, loc := range templateActionRe.FindAllStringSubmatchIndex(line, -1) { + inner := strings.TrimSpace(line[loc[2]:loc[3]]) + + switch firstToken(inner) { + case "end": + if len(stack) > 0 { + stack = stack[:len(stack)-1] + } + + continue + case "else": + continue + case "if": + stack = append(stack, arrayScope{rebindDot: false}) + continue + case "with", "define", "block": + stack = append(stack, arrayScope{rebindDot: true}) + continue + case "range": + stack = append(stack, newRangeScope(inner, prefix, risky, r)) + continue + } + + tracked := emissionScope(leadingCommand(inner), innermostDotScope, findVarScope) + if tracked == nil { + continue + } + + if inBlock[i] || pipelineIsSafe(inner) { + continue + } + + if standalone, quoted := analyzeContext(line[:loc[0]], line[loc[1]:]); !standalone || quoted { + continue + } + + errorList.WithFilePath(relPath).WithLineNumber(i+1). + Errorf("array element from '.Values.%s.%s' is an OpenAPI string without a validation pattern (pattern/enum/format) and must be quoted in templates: use '| quote' or wrap the value in quotes", + valuesKey, tracked.arrPath) + } + } +} + +// newRangeScope builds the scope for a `range` action, marking it as a tracked risky +// array iteration when it ranges over a risky array path that is not excluded. +func newRangeScope(inner, prefix string, risky *riskyPaths, rule *OpenAPIValuesQuoteRule) arrayScope { + sc := arrayScope{rebindDot: true} + + coll, elemVar := parseRange(inner) + + collCmd := firstToken(coll) + if !strings.HasPrefix(collCmd, prefix) { + return sc + } + + arr := collCmd[len(prefix):] + if _, ok := risky.array[arr]; !ok || !rule.Enabled(arr) { + return sc + } + + sc.elemVar = elemVar + sc.arrPath = arr + + return sc +} + +// emissionScope returns the tracked range scope an emission of cmd refers to, or nil. +// A bare dot resolves to the innermost dot-rebinding scope; a `$var` resolves to the +// nearest enclosing scope that introduced it. +func emissionScope(cmd string, innermostDot func() *arrayScope, findVar func(string) *arrayScope) *arrayScope { + switch { + case cmd == ".": + if s := innermostDot(); s != nil && s.elemVar == "." { + return s + } + case strings.HasPrefix(cmd, "$"): + if s := findVar(cmd); s != nil && s.elemVar == cmd { + return s + } + } + + return nil +} + +// parseRange splits a `range ...` action into the collection expression and the loop +// element variable. `range $i, $e := X` -> ("X", "$e"); `range $e := X` -> ("X", "$e"); +// `range X` -> ("X", "."). +func parseRange(inner string) (string, string) { + rest := strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(inner), "range")) + + if left, right, found := strings.Cut(rest, ":="); found { + vars := strings.Split(left, ",") + return strings.TrimSpace(right), strings.TrimSpace(vars[len(vars)-1]) + } + + return rest, "." +} + +// leadingCommand returns the first token of the first stage of a template pipeline — +// the command that produces the value (`.Values.x`, `if`, `range`, `$v`, `quote`, …). +func leadingCommand(inner string) string { + stages := splitPipeline(inner) + if len(stages) == 0 { + return "" + } + + return firstToken(stages[0]) +} + +// pipelineIsSafe reports whether any function stage of the pipeline is known to emit +// a YAML-safe scalar (see safeFuncs). +func pipelineIsSafe(inner string) bool { + stages := splitPipeline(inner) + for _, stage := range stages[1:] { + if _, ok := safeFuncs[firstToken(stage)]; ok { + return true + } + } + + return false +} + +// splitPipeline splits a template expression on top-level `|`, ignoring pipes inside +// quotes or parentheses. +func splitPipeline(inner string) []string { + var ( + stages []string + start int + depth int + quote byte + ) + + for i := 0; i < len(inner); i++ { + c := inner[i] + switch { + case quote != 0: + if c == quote { + quote = 0 + } + case c == '"' || c == '\'' || c == '`': + quote = c + case c == '(': + depth++ + case c == ')': + if depth > 0 { + depth-- + } + case c == '|' && depth == 0: + stages = append(stages, strings.TrimSpace(inner[start:i])) + start = i + 1 + } + } + + return append(stages, strings.TrimSpace(inner[start:])) +} + +// firstToken returns the first whitespace-separated token of s. +func firstToken(s string) string { + s = strings.TrimSpace(s) + if idx := strings.IndexAny(s, " \t"); idx >= 0 { + return s[:idx] + } + + return s +} + +// analyzeContext inspects the text around an action to decide whether the action is +// the entire YAML value (standalone) and, if so, whether it is wrapped in matching +// quotes. Non-standalone actions (embedded in a larger scalar, flow collections, …) +// return standalone=false and are skipped to avoid false positives. +func analyzeContext(pre, post string) (bool, bool) { + post = strings.TrimRight(stripTrailingComment(post), " \t") + + var closeQuote byte + + if rest := strings.TrimLeft(post, " \t"); rest != "" { + if rest[0] != '"' && rest[0] != '\'' { + return false, false + } + + closeQuote = rest[0] + + if strings.TrimSpace(rest[1:]) != "" { + return false, false + } + } + + m := standaloneOpenRe.FindStringSubmatch(pre) + if m == nil { + return false, false + } + + var openQuote byte + if m[3] != "" { + openQuote = m[3][0] + } + + switch { + case openQuote == 0 && closeQuote == 0: + return true, false + case openQuote != 0 && openQuote == closeQuote: + return true, true + default: + // A quote on only one side is ambiguous; skip conservatively. + return false, false + } +} + +// stripTrailingComment removes a YAML inline comment (a `#` preceded by whitespace or +// at the start) from s. +func stripTrailingComment(s string) string { + for i := 0; i < len(s); i++ { + if s[i] == '#' && (i == 0 || s[i-1] == ' ' || s[i-1] == '\t') { + return s[:i] + } + } + + return s +} + +// computeBlockScalarLines marks, for each line, whether it is inside a YAML +// block-scalar (literal/folded) body, where substitutions are part of a string and +// need no quoting. +func computeBlockScalarLines(lines []string) []bool { + res := make([]bool, len(lines)) + + inBlock := false + blockIndent := 0 + + for i, line := range lines { + trimmed := strings.TrimSpace(line) + indent := len(line) - len(strings.TrimLeft(line, " \t")) + + if inBlock { + if trimmed == "" || indent > blockIndent { + res[i] = true + continue + } + + inBlock = false + } + + if trimmed != "" && blockScalarHeaderRe.MatchString(line) { + inBlock = true + blockIndent = indent + } + } + + return res +} + +// schemaWalker traverses an OpenAPI value schema and records the paths whose type +// resolves to an unvalidated string. +type schemaWalker struct { + root map[string]any + out *riskyPaths +} + +func (w *schemaWalker) walk(path string, schema map[string]any, depth int) { + if schema == nil || depth > 64 { + return + } + + schema = w.resolveRef(schema) + + // Composition: walk every branch at the same path. A branch that is itself an + // unvalidated string registers the path (union for oneOf/anyOf; for allOf the + // merged validation keywords are evaluated per-branch, which is conservative). + for _, key := range []string{"allOf", "oneOf", "anyOf"} { + if subs, ok := schema[key].([]any); ok { + for _, sub := range subs { + if sm, ok := sub.(map[string]any); ok { + w.walk(path, sm, depth+1) + } + } + } + } + + types := schemaTypes(schema) + + if props, ok := schema["properties"].(map[string]any); ok { + for name, raw := range props { + if ps, ok := raw.(map[string]any); ok { + w.walk(joinPath(path, name), ps, depth+1) + } + } + } + + if types["array"] && path != "" { + if items, ok := schema["items"].(map[string]any); ok { + resolved := w.resolveRef(items) + if schemaTypes(resolved)["string"] && !hasValidation(resolved) { + w.out.array[path] = struct{}{} + } + } + } + + if types["string"] && path != "" && !hasValidation(schema) { + w.out.scalar[path] = struct{}{} + } +} + +// resolveRef resolves a local `$ref` ("#/...") chain, merging sibling keys of the +// referring schema over the target. A non-local or unresolvable ref is returned as-is. +func (w *schemaWalker) resolveRef(schema map[string]any) map[string]any { + for range 16 { + ref, ok := schema["$ref"].(string) + if !ok { + return schema + } + + if !strings.HasPrefix(ref, "#/") { + return schema + } + + target := w.root + resolved := true + + for part := range strings.SplitSeq(strings.TrimPrefix(ref, "#/"), "/") { + part = strings.ReplaceAll(strings.ReplaceAll(part, "~1", "/"), "~0", "~") + + next, ok := target[part].(map[string]any) + if !ok { + resolved = false + + break + } + + target = next + } + + if !resolved { + return schema + } + + merged := make(map[string]any, len(target)+len(schema)) + maps.Copy(merged, target) + + for k, v := range schema { + if k != "$ref" { + merged[k] = v + } + } + + schema = merged + } + + return schema +} + +// schemaTypes returns the set of declared JSON-schema types (handles both a plain +// `type: string` and the nullable `type: [string, "null"]` form). +func schemaTypes(schema map[string]any) map[string]bool { + res := make(map[string]bool) + + switch t := schema["type"].(type) { + case string: + res[t] = true + case []any: + for _, v := range t { + if s, ok := v.(string); ok { + res[s] = true + } + } + } + + return res +} + +// hasValidation reports whether the schema constrains a string enough that it never +// needs quoting: a pattern, an enum, or a format. +func hasValidation(schema map[string]any) bool { + if s, ok := schema["pattern"].(string); ok && strings.TrimSpace(s) != "" { + return true + } + + if enum, ok := schema["enum"].([]any); ok && len(enum) > 0 { + return true + } + + if f, ok := schema["format"].(string); ok && strings.TrimSpace(f) != "" { + return true + } + + return false +} + +func joinPath(base, key string) string { + if base == "" { + return key + } + + return base + "." + key +} diff --git a/pkg/linters/templates/rules/openapi_values_quote_test.go b/pkg/linters/templates/rules/openapi_values_quote_test.go new file mode 100644 index 00000000..61ff2f0a --- /dev/null +++ b/pkg/linters/templates/rules/openapi_values_quote_test.go @@ -0,0 +1,433 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rules + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/gojuno/minimock/v3" + + "github.com/deckhouse/dmt/internal/mocks" + "github.com/deckhouse/dmt/internal/modules/values" + "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/errors" +) + +const testModuleName = "test-mod" + +// configValuesObject is a minimal config-values.yaml that adds no properties of its own. +const configValuesObject = "type: object\nproperties: {}\n" + +func TestOpenAPIValuesQuoteRule(t *testing.T) { + valuesKey := values.ModuleCamelName(testModuleName) // "testMod" + + tests := []struct { + name string + valuesSchema string + files map[string]string + excludes []string + wantCount int + wantContains []string + wantNotExists string + }{ + { + name: "unquoted scalar string is flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ .Values." + valuesKey + ".foo }}\n", + }, + wantCount: 1, + wantContains: []string{"foo", "must be quoted"}, + }, + { + name: "value piped through quote is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ .Values." + valuesKey + ".foo | quote }}\n", + }, + wantCount: 0, + }, + { + name: "value wrapped in double quotes is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: \"{{ .Values." + valuesKey + ".foo }}\"\n", + }, + wantCount: 0, + }, + { + name: "value wrapped in single quotes is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: '{{ .Values." + valuesKey + ".foo }}'\n", + }, + wantCount: 0, + }, + { + name: "string with pattern is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string + pattern: '^[a-z]+$' +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ .Values." + valuesKey + ".foo }}\n", + }, + wantCount: 0, + }, + { + name: "string with enum is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string + enum: ["A", "B"] +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ .Values." + valuesKey + ".foo }}\n", + }, + wantCount: 0, + }, + { + name: "string with format is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string + format: date-time +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ .Values." + valuesKey + ".foo }}\n", + }, + wantCount: 0, + }, + { + name: "integer is not flagged", + valuesSchema: `type: object +properties: + foo: + type: integer +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ .Values." + valuesKey + ".foo }}\n", + }, + wantCount: 0, + }, + { + name: "nested string path is flagged", + valuesSchema: `type: object +properties: + foo: + type: object + properties: + bar: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n bar: {{ .Values." + valuesKey + ".foo.bar }}\n", + }, + wantCount: 1, + wantContains: []string{"foo.bar"}, + }, + { + name: "nullable string is flagged", + valuesSchema: `type: object +properties: + foo: + type: ["string", "null"] +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ .Values." + valuesKey + ".foo }}\n", + }, + wantCount: 1, + wantContains: []string{"foo"}, + }, + { + name: "string via $ref is flagged", + valuesSchema: `type: object +properties: + foo: + $ref: '#/definitions/nameType' +definitions: + nameType: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ .Values." + valuesKey + ".foo }}\n", + }, + wantCount: 1, + wantContains: []string{"foo"}, + }, + { + name: "string via allOf is flagged", + valuesSchema: `type: object +properties: + foo: + allOf: + - type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ .Values." + valuesKey + ".foo }}\n", + }, + wantCount: 1, + wantContains: []string{"foo"}, + }, + { + name: "string branch of oneOf is flagged", + valuesSchema: `type: object +properties: + foo: + oneOf: + - type: string + - type: integer +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ .Values." + valuesKey + ".foo }}\n", + }, + wantCount: 1, + wantContains: []string{"foo"}, + }, + { + name: "unquoted array element via range is flagged", + valuesSchema: `type: object +properties: + list: + type: array + items: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "items:\n{{- range .Values." + valuesKey + ".list }}\n - {{ . }}\n{{- end }}\n", + }, + wantCount: 1, + wantContains: []string{"array element", "list"}, + }, + { + name: "quoted array element via range is not flagged", + valuesSchema: `type: object +properties: + list: + type: array + items: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "items:\n{{- range .Values." + valuesKey + ".list }}\n - {{ . | quote }}\n{{- end }}\n", + }, + wantCount: 0, + }, + { + name: "unquoted array element via named range variable is flagged", + valuesSchema: `type: object +properties: + list: + type: array + items: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "items:\n{{- range $x := .Values." + valuesKey + ".list }}\n - {{ $x }}\n{{- end }}\n", + }, + wantCount: 1, + wantContains: []string{"array element", "list"}, + }, + { + name: "array of strings with pattern is not flagged", + valuesSchema: `type: object +properties: + list: + type: array + items: + type: string + pattern: '^[a-z]+$' +`, + files: map[string]string{ + "templates/cm.yaml": "items:\n{{- range .Values." + valuesKey + ".list }}\n - {{ . }}\n{{- end }}\n", + }, + wantCount: 0, + }, + { + name: "value embedded in a larger scalar is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: prefix-{{ .Values." + valuesKey + ".foo }}\n", + }, + wantCount: 0, + }, + { + name: "value inside a block scalar is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n script: |\n echo {{ .Values." + valuesKey + ".foo }}\n", + }, + wantCount: 0, + }, + { + name: "value used in an if condition is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "{{- if .Values." + valuesKey + ".foo }}\ndata: {}\n{{- end }}\n", + }, + wantCount: 0, + }, + { + name: "excluded value path is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ .Values." + valuesKey + ".foo }}\n", + }, + excludes: []string{"foo"}, + wantCount: 0, + }, + { + name: "value piped through b64enc is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ .Values." + valuesKey + ".foo | b64enc }}\n", + }, + wantCount: 0, + }, + { + name: "unquoted value with default is flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ .Values." + valuesKey + `.foo | default "x" }}` + "\n", + }, + wantCount: 1, + wantContains: []string{"foo"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + modulePath := t.TempDir() + + writeFile(t, modulePath, "openapi/values.yaml", tt.valuesSchema) + writeFile(t, modulePath, "openapi/config-values.yaml", configValuesObject) + + for rel, content := range tt.files { + writeFile(t, modulePath, rel, content) + } + + mc := minimock.NewController(t) + module := mocks.NewModuleMock(mc) + module.GetPathMock.Return(modulePath) + module.GetNameMock.Return(testModuleName) + + errorList := errors.NewLintRuleErrorsList() + + excludes := make([]pkg.StringRuleExclude, 0, len(tt.excludes)) + for _, e := range tt.excludes { + excludes = append(excludes, pkg.StringRuleExclude(e)) + } + + NewOpenAPIValuesQuoteRule(excludes).CheckStringValuesQuoted(module, errorList) + + errs := errorList.GetErrors() + if len(errs) != tt.wantCount { + t.Fatalf("expected %d finding(s), got %d: %s", tt.wantCount, len(errs), formatErrs(errs)) + } + + for _, want := range tt.wantContains { + if !anyContains(errs, want) { + t.Errorf("expected a finding containing %q, got: %s", want, formatErrs(errs)) + } + } + }) + } +} + +func writeFile(t *testing.T, root, rel, content string) { + t.Helper() + + full := filepath.Join(root, rel) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatalf("mkdir %s: %v", filepath.Dir(full), err) + } + + if err := os.WriteFile(full, []byte(content), 0o600); err != nil { + t.Fatalf("write %s: %v", full, err) + } +} + +func anyContains(errs []pkg.LinterError, substr string) bool { + for i := range errs { + if strings.Contains(errs[i].Text, substr) { + return true + } + } + + return false +} + +func formatErrs(errs []pkg.LinterError) string { + var b strings.Builder + for i := range errs { + b.WriteString("\n - ") + b.WriteString(errs[i].Text) + } + + if b.Len() == 0 { + return "(none)" + } + + return b.String() +} diff --git a/pkg/linters/templates/templates.go b/pkg/linters/templates/templates.go index b673aea5..f72b3ec4 100644 --- a/pkg/linters/templates/templates.go +++ b/pkg/linters/templates/templates.go @@ -111,6 +111,10 @@ func (l *Templates) Run(m *modules.Module) { // HelmRender rule rules.NewHelmRenderRule().Check(m, errorList.WithMaxLevel(l.cfg.Rules.HelmRenderRule.GetLevel())) + + // OpenAPIValuesQuote rule: string values without a validation pattern must be quoted in templates + rules.NewOpenAPIValuesQuoteRule(l.cfg.ExcludeRules.OpenAPIValuesQuote.Get()). + CheckStringValuesQuoted(m, errorList.WithMaxLevel(l.cfg.Rules.OpenAPIValuesQuoteRule.GetLevel())) } func (l *Templates) Name() string { diff --git a/test/e2e/testdata/templates/openapi-values-quote/expected.yaml b/test/e2e/testdata/templates/openapi-values-quote/expected.yaml new file mode 100644 index 00000000..de3c817a --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote/expected.yaml @@ -0,0 +1,15 @@ +description: > + A module template that renders a pattern-less OpenAPI string value unquoted must be + flagged by the openapi-values-quote rule; a string value constrained by a pattern + must not be. +module: module +expect: + - linter: templates + rule: openapi-values-quote + level: error + textContains: "greeting" + count: 1 +expectAbsent: + - linter: templates + rule: openapi-values-quote + textContains: "code" diff --git a/test/e2e/testdata/templates/openapi-values-quote/module/module.yaml b/test/e2e/testdata/templates/openapi-values-quote/module/module.yaml new file mode 100644 index 00000000..c4cebdab --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote/module/module.yaml @@ -0,0 +1,2 @@ +name: e2e-quote +namespace: e2e-quote diff --git a/test/e2e/testdata/templates/openapi-values-quote/module/openapi/config-values.yaml b/test/e2e/testdata/templates/openapi-values-quote/module/openapi/config-values.yaml new file mode 100644 index 00000000..e5b0ab30 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote/module/openapi/config-values.yaml @@ -0,0 +1,9 @@ +type: object +properties: + greeting: + type: string + default: hello + code: + type: string + pattern: '^[a-z]+$' + default: abc diff --git a/test/e2e/testdata/templates/openapi-values-quote/module/openapi/values.yaml b/test/e2e/testdata/templates/openapi-values-quote/module/openapi/values.yaml new file mode 100644 index 00000000..47180da5 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote/module/openapi/values.yaml @@ -0,0 +1,4 @@ +x-extend: + schema: config-values.yaml +type: object +properties: {} diff --git a/test/e2e/testdata/templates/openapi-values-quote/module/templates/configmap.yaml b/test/e2e/testdata/templates/openapi-values-quote/module/templates/configmap.yaml new file mode 100644 index 00000000..9bad874a --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote/module/templates/configmap.yaml @@ -0,0 +1,10 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-quote + namespace: e2e-quote +data: + # pattern-less string rendered unquoted -> must be flagged + greeting: {{ .Values.e2eQuote.greeting }} + # string with a validation pattern -> must NOT be flagged + code: {{ .Values.e2eQuote.code }} From 0881f80db54b071fd703f58ac5f7820b599833f0 Mon Sep 17 00:00:00 2001 From: Pavel Okhlopkov Date: Wed, 19 Aug 2026 22:38:17 +0300 Subject: [PATCH 2/8] bump Signed-off-by: Pavel Okhlopkov --- pkg/linters/templates/README.md | 32 +- .../templates/rules/openapi_values_quote.go | 1119 +++++++++++++++-- .../rules/openapi_values_quote_test.go | 1094 +++++++++++++++- .../openapi-values-quote-forms/expected.yaml | 113 ++ .../module/module.yaml | 2 + .../module/openapi/config-values.yaml | 100 ++ .../module/openapi/values.yaml | 4 + .../module/templates/_helpers.tpl | 14 + .../module/templates/flags.yaml | 50 + .../module/templates/safe.yaml | 12 + .../openapi-values-quote-refs/expected.yaml | 30 + .../module/module.yaml | 2 + .../module/openapi/config-values.yaml | 28 + .../module/openapi/values.yaml | 4 + .../module/templates/configmap.yaml | 11 + .../openapi-values-quote/expected.yaml | 92 +- .../module/openapi/config-values.yaml | 101 ++ .../module/templates/_helpers.tpl | 3 + .../module/templates/args.yaml | 20 + .../module/templates/configmap.yaml | 29 +- .../module/templates/include.yaml | 8 + .../module/templates/maps-and-with.yaml | 15 + .../module/templates/servers.yaml | 17 + .../module/templates/with.yaml | 10 + 24 files changed, 2774 insertions(+), 136 deletions(-) create mode 100644 test/e2e/testdata/templates/openapi-values-quote-forms/expected.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-forms/module/module.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-forms/module/openapi/config-values.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-forms/module/openapi/values.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-forms/module/templates/_helpers.tpl create mode 100644 test/e2e/testdata/templates/openapi-values-quote-forms/module/templates/flags.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-forms/module/templates/safe.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-refs/expected.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-refs/module/module.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-refs/module/openapi/config-values.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-refs/module/openapi/values.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-refs/module/templates/configmap.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote/module/templates/_helpers.tpl create mode 100644 test/e2e/testdata/templates/openapi-values-quote/module/templates/args.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote/module/templates/include.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote/module/templates/maps-and-with.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote/module/templates/servers.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote/module/templates/with.yaml diff --git a/pkg/linters/templates/README.md b/pkg/linters/templates/README.md index 8e0ff4dd..fcd74932 100644 --- a/pkg/linters/templates/README.md +++ b/pkg/linters/templates/README.md @@ -2043,21 +2043,33 @@ linters-settings: **Description:** -The rule reads the module value schema (`openapi/values.yaml` and `openapi/config-values.yaml`), collects every value path whose type resolves to a **string with no `pattern`, `enum`, or `format`**, and then checks the module `templates/` for usages of those values (`.Values..`). A usage that is rendered as a bare YAML value — not piped through `quote`/`squote` (or another YAML-safe function such as `b64enc`, `toJson`, `sha256sum`), and not wrapped in quotes — is reported. +The rule reads the module value schema (`openapi/values.yaml` and `openapi/config-values.yaml`), collects every value path whose type resolves to a **string with no `pattern`, `enum`, or `format`**, and then checks the module `templates/` for usages of those values. A usage that is rendered as a bare YAML value — not piped through a YAML-safe function, and not wrapped in quotes — is reported. + +Both the direct form `.Values..` and the root-scoped `$.Values..` form (used inside `range`/`with` blocks) are recognised. **What counts as a string:** - a direct `type: string` property; - a nullable string (`type: ["string", "null"]`); - a string reached through `$ref`, `allOf`, `oneOf`, or `anyOf`; -- items of a string array (`type: array` with `items: {type: string}`), checked through `range`. +- items of a string array (`type: array` with `items: {type: string}`), checked through `range` (`{{ range … }}{{ . }}{{ end }}`); +- a string sub-field (at any depth) of an array-of-objects element, checked through a named range: `{{ range $s := .Values.mod.servers }}{{ $s.host }}{{ end }}` (and the dot-binding form `{{ range .Values.mod.servers }}{{ .host }}{{ end }}`); +- an element of a string-array sub-field, checked through a nested range (range-in-range) at any nesting depth: `{{ range $s := .Values.mod.servers }}{{ range $a := $s.aliases }}{{ $a }}{{ end }}{{ end }}`; +- values of a string map (`type: object` with `additionalProperties: {type: string}`), checked through `{{ range $k, $v := .Values.mod.labels }}{{ $v }}{{ end }}` (map-of-objects and nested maps too); +- elements of an array of string arrays (`items: {type: array, items: {type: string}}`), checked through a nested range; +- a string scoped by `with`, both over a scalar (`{{ with .Values.mod.foo }}{{ . }}{{ end }}`) and over a single object (`{{ with .Values.mod.db }}{{ .host }}{{ end }}`); +- a value copied into a template variable and then emitted (`{{ $x := .Values.mod.foo }}… {{ $x }}`), including aliases of loop element variables and an array bound to a variable and later ranged (`{{ $x := .Values.mod.list }}{{ range $x }}{{ . }}{{ end }}`); +- a value scoped by `with` over an object **variable** (`{{ with $s }}{{ .host }}{{ end }}`); +- a value passed to a **module-defined** template that renders it unquoted (`{{ include "mymod.env" .Values.mod.config }}` where `define "mymod.env"` emits `{{ .value }}` or ranges `{{ range .items }}{{ . }}{{ end }}` unquoted), followed transitively across the module's own `define`/`include` chain. External templates (helm_lib, …) are not inspected, so this never fires on them. **What makes a string safe (not reported):** -- it declares a `pattern`, an `enum`, or a `format` in the schema; **or** -- every template usage quotes it (`| quote`, `| squote`, or literal `"…"`/`'…'` wrapping), or pipes it through a function whose output is always YAML-safe. +- it declares a `pattern`, an `enum`, or a `format` in the schema (note: `minLength`/`maxLength` do **not** exempt it — they don't restrict the character set); **or** +- every template usage wraps it in literal `"…"`/`'…'`, or pipes it through a function whose output is always YAML-safe: `quote`, `squote`, `toJson`/`toRawJson`, `toYaml`, `b64enc`/`b32enc`, `sha1sum`/`sha256sum`/`sha512sum`/`adler32sum` (and their `must…` variants), or `printf` with a `%q` verb. + +The rule reports both a **standalone** value (`key: {{ … }}` / `- {{ … }}`, fix: add `| quote`) and a value **embedded** in a larger unquoted scalar (`host: prefix-{{ … }}`, fix: wrap the whole value in quotes) — the message tells you which fix applies. A risky value passed to a common passthrough function — a string transform (`{{ printf "%s" .Values.mod.foo }}`, `{{ upper $v }}`) or an array/map element accessor (`{{ index .Values.mod.list 0 }}`) — is reported as well, for both `.Values` references and variables. A value used only in a condition (`{{ if … }}`) is not a rendered value and is not reported. -To keep findings high-confidence, the rule only reports **standalone** values (`key: {{ … }}` or `- {{ … }}`). A value embedded in a larger scalar (`host: prefix-{{ … }}`), used in a condition (`{{ if … }}`), or placed inside a YAML block scalar (`|` / `>`) is not reported. +**Known limitations (not reported):** a value inside a YAML block scalar (`|` / `>`), where it is already part of a string and must **not** be quoted; a value passed to an **external** template (helm_lib and other charts, whose bodies are not scanned); a map **key** emitted as `{{ $k }}:` (indistinguishable from a numeric array index without extra type info); a value reached through dynamic access (`dig`/`get`/`pluck`, or a `dict` built inline and passed on); and a `{{ … }}` action that spans multiple physical lines. **Why it matters:** @@ -2104,6 +2116,16 @@ args: {{- end }} ``` +For arrays of objects, quote each risky string sub-field of the element: + +```yaml +hosts: +{{- range $s := .Values.myModule.servers }} + - host: {{ $s.host | quote }} # host has no pattern/enum/format -> quote it + zone: {{ $s.zone }} # zone is an enum -> fine as is +{{- end }} +``` + **Configuration:** Set the impact level or exclude specific value paths (matched against the dotted path relative to the module values root, e.g. `foo.bar`): diff --git a/pkg/linters/templates/rules/openapi_values_quote.go b/pkg/linters/templates/rules/openapi_values_quote.go index f9091a1b..801c13da 100644 --- a/pkg/linters/templates/rules/openapi_values_quote.go +++ b/pkg/linters/templates/rules/openapi_values_quote.go @@ -46,6 +46,7 @@ var openAPIValuesFiles = []string{"values.yaml", "config-values.yaml"} var safeFuncs = map[string]struct{}{ "quote": {}, "squote": {}, "toJson": {}, "mustToJson": {}, "toRawJson": {}, "mustToRawJson": {}, + "toYaml": {}, "mustToYaml": {}, "b64enc": {}, "b32enc": {}, "sha1sum": {}, "sha256sum": {}, "sha512sum": {}, "adler32sum": {}, } @@ -59,10 +60,49 @@ var templateActionRe = regexp.MustCompile(`{{-?\s*(.*?)\s*-?}}`) // substitutions there are already part of a string and must not be quoted. var blockScalarHeaderRe = regexp.MustCompile(`^(\s*)(-\s+)?([\w.\-/]+\s*:\s*)?[|>][+-]?\d*\s*(#.*)?$`) -// standaloneOpenRe matches the text before an action when the action is the whole -// YAML value: indentation, an optional list dash, an optional `key:`, and an -// optional opening quote (captured). -var standaloneOpenRe = regexp.MustCompile(`^\s*(-\s+)?([\w.\-/]+\s*:\s*)?(["']?)\s*$`) +// assignRe matches a template variable assignment `$name := expr` / `$name = expr`. +var assignRe = regexp.MustCompile(`^(\$[A-Za-z_]\w*)\s*(:?=)\s*(.+)$`) + +// includeRe matches an `include "name" arg` / `template "name" arg` action, capturing +// the called template name and the argument expression. +var includeRe = regexp.MustCompile(`^(?:include|template)\s+"([^"]+)"\s*(.*)$`) + +// defineNameRe captures the name of a `define "name"` block. +var defineNameRe = regexp.MustCompile(`^define\s+"([^"]+)"`) + +// valuesRefRe finds a `.Values.<...>` / `$.Values.<...>` reference token, used to spot +// a risky value passed as a function argument (`{{ printf "%s" .Values.mod.foo }}`). +var valuesRefRe = regexp.MustCompile(`\$?\.Values\.[A-Za-z0-9_.]+`) + +// varTokenRe finds a variable or dot-relative reference token (`$v`, `$s.host`, +// `.field`), used to spot a risky variable passed as a function argument. +var varTokenRe = regexp.MustCompile(`\$[A-Za-z_]\w*(?:\.[\w.]+)?|\.[A-Za-z_][\w.]*`) + +// passthroughFuncs are functions whose output is (or contains) their string argument +// unchanged as far as YAML safety goes — so a risky value passed to one still needs +// quoting. The set is deliberately curated; an unknown leading function is left alone +// to avoid false positives. +var passthroughFuncs = map[string]struct{}{ + "printf": {}, "print": {}, "println": {}, + "default": {}, "coalesce": {}, "cat": {}, "toString": {}, + "upper": {}, "lower": {}, "title": {}, "untitle": {}, "nospace": {}, + "trim": {}, "trimAll": {}, "trimPrefix": {}, "trimSuffix": {}, + "replace": {}, "repeat": {}, "trunc": {}, "substr": {}, "abbrev": {}, + "snakecase": {}, "camelcase": {}, "kebabcase": {}, "swapcase": {}, +} + +// elementFuncs return an element (or the value) of their array/map argument, so a risky +// array/map argument yields a risky string result: `{{ index .Values.mod.list 0 }}`. +var elementFuncs = map[string]struct{}{ + "index": {}, "first": {}, "last": {}, "mustFirst": {}, "mustLast": {}, +} + +// valuePreRe splits the text before an action into the YAML plumbing (indentation, an +// optional list dash, an optional `key:` — a literal key or a template-action key such +// as `{{ $k }}:` from a map range) and the value text that precedes the action +// (captured group 3), which is empty for a standalone value and non-empty when the +// action is embedded in a larger scalar. +var valuePreRe = regexp.MustCompile(`^\s*(-\s+)?((?:\{\{[^{}]*\}\}|[\w.\-/]+)\s*:\s*)?(.*)$`) // OpenAPIValuesQuoteRule flags template usages of module OpenAPI string values that // have no validation pattern (no pattern/enum/format) and are rendered unquoted. @@ -84,23 +124,30 @@ func NewOpenAPIValuesQuoteRule(excludeRules []pkg.StringRuleExclude) *OpenAPIVal } } -// riskyPaths holds the value paths (relative to the module values root) whose type -// resolves to an unvalidated string. scalar paths are string leaves; array paths are -// arrays whose items are unvalidated strings (checked through `range`). +// riskyPaths holds the value paths (relative to a schema level) whose type resolves to +// an unvalidated string. scalar paths are string leaves; array paths are arrays whose +// items are unvalidated strings (checked through `range`); object maps an +// array-of-objects path to the recursive risk profile of its element (checked through +// `range $x := …` and `{{ $x.field }}`, including a nested `range` over a string-array +// sub-field). The structure is recursive so any depth of range-in-range is covered. type riskyPaths struct { - scalar map[string]struct{} - array map[string]struct{} + scalar map[string]struct{} + array map[string]struct{} + object map[string]*riskyPaths + arrayArray map[string]struct{} } func newRiskyPaths() *riskyPaths { return &riskyPaths{ - scalar: make(map[string]struct{}), - array: make(map[string]struct{}), + scalar: make(map[string]struct{}), + array: make(map[string]struct{}), + object: make(map[string]*riskyPaths), + arrayArray: make(map[string]struct{}), } } func (p *riskyPaths) empty() bool { - return len(p.scalar) == 0 && len(p.array) == 0 + return len(p.scalar) == 0 && len(p.array) == 0 && len(p.object) == 0 && len(p.arrayArray) == 0 } // CheckStringValuesQuoted parses the module OpenAPI value schema, collects the @@ -140,18 +187,33 @@ func (r *OpenAPIValuesQuoteRule) CheckStringValuesQuoted(m pkg.Module, errorList files := fsutils.GetFiles(templatesPath, true, fsutils.FilterFileByExtensions(".yaml", ".yml", ".tpl", ".tpl.yaml", ".tpl.yml")) + type templateFile struct { + relPath string + lines []string + } + + parsed := make([]templateFile, 0, len(files)) + contents := make([]string, 0, len(files)) + for _, filePath := range files { content, err := os.ReadFile(filePath) if err != nil { continue } - relPath := fsutils.Rel(modulePath, filePath) - lines := strings.Split(string(content), "\n") - inBlock := computeBlockScalarLines(lines) + parsed = append(parsed, templateFile{fsutils.Rel(modulePath, filePath), strings.Split(string(content), "\n")}) + contents = append(contents, string(content)) + } - r.checkScalars(relPath, lines, inBlock, valuesKey, risky, errorList) - r.checkArrayRanges(relPath, lines, inBlock, valuesKey, risky, errorList) + // Cross-template flow: what each module `define` renders from its parameter, so a + // risky value passed to it via include/template can be checked at the call site. + defineEmits := resolveDefineEmits(collectDefines(contents)) + + for _, f := range parsed { + inBlock := computeBlockScalarLines(f.lines) + + r.checkScalars(f.relPath, f.lines, inBlock, valuesKey, risky, errorList) + r.checkScopedEmissions(f.relPath, f.lines, inBlock, valuesKey, risky, defineEmits, errorList) } } @@ -160,8 +222,6 @@ func (r *OpenAPIValuesQuoteRule) checkScalars( relPath string, lines []string, inBlock []bool, valuesKey string, risky *riskyPaths, errorList *errors.LintRuleErrorsList, ) { - prefix := ".Values." + valuesKey + "." - for i, line := range lines { if inBlock[i] { continue @@ -170,17 +230,8 @@ func (r *OpenAPIValuesQuoteRule) checkScalars( for _, loc := range templateActionRe.FindAllStringSubmatchIndex(line, -1) { inner := strings.TrimSpace(line[loc[2]:loc[3]]) - cmd := leadingCommand(inner) - if !strings.HasPrefix(cmd, prefix) { - continue - } - - valPath := cmd[len(prefix):] - if _, ok := risky.scalar[valPath]; !ok { - continue - } - - if !r.Enabled(valPath) { + valPath, ok := scalarEmission(inner, valuesKey, risky) + if !ok || !r.Enabled(valPath) { continue } @@ -188,60 +239,127 @@ func (r *OpenAPIValuesQuoteRule) checkScalars( continue } - if standalone, quoted := analyzeContext(line[:loc[0]], line[loc[1]:]); !standalone || quoted { + report, wrap := analyzeContext(line[:loc[0]], line[loc[1]:]) + if !report { continue } errorList.WithFilePath(relPath).WithLineNumber(i+1). - Errorf("value '.Values.%s.%s' is an OpenAPI string without a validation pattern (pattern/enum/format) and must be quoted in templates: use '| quote' or wrap the value in quotes", - valuesKey, valPath) + Errorf("value '.Values.%s.%s' is an OpenAPI string without a validation pattern (pattern/enum/format) %s", + valuesKey, valPath, quoteAdvice(wrap)) } } } -// arrayScope tracks a template block while scanning for `range` loops over risky -// string arrays. elemVar is the loop element variable ("$x" or "." for dot-binding) -// when the range iterates a risky array; it is empty for any other block. -type arrayScope struct { - rebindDot bool - elemVar string - arrPath string +// quoteAdvice tailors the fix hint: an embedded value must be wrapped as a whole, +// while a standalone value can also be piped through quote. +func quoteAdvice(wrap bool) string { + if wrap { + return "and must be quoted in templates: wrap the whole value in quotes" + } + + return "and must be quoted in templates: use '| quote' or wrap the value in quotes" } -// checkArrayRanges reports unquoted emissions of elements while ranging over a risky -// string array (`{{- range $e := .Values.mod.list }}{{ $e }}{{- end }}`). -func (r *OpenAPIValuesQuoteRule) checkArrayRanges( - relPath string, lines []string, inBlock []bool, valuesKey string, - risky *riskyPaths, errorList *errors.LintRuleErrorsList, -) { - if len(risky.array) == 0 { - return +// scalarEmission returns the risky scalar value path a direct emission +// (`{{ .Values.mod.foo }}`, or the parenthesised `{{ (.Values.mod.foo) }}`) renders, +// and whether it matched. Passthrough-function forms are handled in checkScopedEmissions +// (which also sees variables). +func scalarEmission(inner, valuesKey string, risky *riskyPaths) (string, bool) { + if p, ok := valuePath(leadingCommand(inner), valuesKey); ok { + return p, has(risky.scalar, p) } - prefix := ".Values." + valuesKey + "." + return "", false +} - var stack []arrayScope +// passthroughRisky returns the risky scalar value path passed as an argument to a +// passthrough function (`{{ printf "%s" .Values.mod.foo }}`, `{{ upper $v }}`), whether +// via a `.Values` reference or a variable, and whether it matched. +func passthroughRisky(inner, valuesKey string, risky *riskyPaths, innermostDot func() *loopScope, findVar func(string) *loopScope) (string, bool) { + lead := leadingCommand(inner) + if lead == "printf" && strings.Contains(inner, "%q") { + return "", false // %q already quotes its output + } - innermostDotScope := func() *arrayScope { - for i := len(stack) - 1; i >= 0; i-- { - if stack[i].rebindDot { - return &stack[i] + // index/first/last yield an element of their array/map argument. + if _, isElem := elementFuncs[lead]; isElem { + for _, tok := range valuesRefRe.FindAllString(inner, -1) { + if p, ok := valuePath(tok, valuesKey); ok && has(risky.array, p) { + return p, true } } - return nil + return "", false } - findVarScope := func(v string) *arrayScope { - for i := len(stack) - 1; i >= 0; i-- { - if stack[i].elemVar == v { - return &stack[i] - } + for _, tok := range valuesRefRe.FindAllString(inner, -1) { + if p, ok := valuePath(tok, valuesKey); ok && has(risky.scalar, p) { + return p, true } + } - return nil + for _, tok := range varTokenRe.FindAllString(inner, -1) { + if scope, sub := emissionScope(tok, innermostDot, findVar); scope != nil { + return emissionPath(scope, sub), true + } } + return "", false +} + +// isPassthrough reports whether a leading function forwards a risky argument to its +// output (a string transform, or an array/map element accessor). +func isPassthrough(lead string) bool { + return has(passthroughFuncs, lead) || has(elementFuncs, lead) +} + +// emissionPath renders the display value path for a tracked emission. +func emissionPath(scope *loopScope, sub string) string { + if scope.kind != "object" { + return scope.valPath + } + + sep := "." + if scope.elemArray { + sep = "[]." + } + + return scope.valPath + sep + sub +} + +// loopScope tracks a template block while scanning for `range`/`with` over risky +// string values. When the block selects a risky value, kind is "array" (string-array +// element), "strarray" (an element that is itself a string array, for range-in-range), +// "object" (an object whose profile holds its risky sub-fields), or "scalar" (a +// `with`-scoped string); elemVar is the element accessor ("$x", or "." for +// dot-binding). kind is empty for any other block. valPath is the display path used in +// findings. elemArray marks an object whose sub-fields belong to array elements +// (`servers[].host`) rather than to a single object (`db.host`). +type loopScope struct { + rebindDot bool + kind string + elemVar string + valPath string + elemArray bool + profile *riskyPaths +} + +// checkScopedEmissions reports unquoted emissions of a risky string reached through a +// `range` over a string array (`{{ range $e := .Values.mod.list }}{{ $e }}`) or a +// `with` over a string scalar (`{{ with .Values.mod.foo }}{{ . }}`), including the +// root-scoped `$.Values....` form used inside such blocks. +func (r *OpenAPIValuesQuoteRule) checkScopedEmissions( + relPath string, lines []string, inBlock []bool, valuesKey string, + risky *riskyPaths, defineEmits map[string]defineEmit, errorList *errors.LintRuleErrorsList, +) { + var stack []loopScope + + assigned := map[string]loopScope{} + + innermostDotScope := func() *loopScope { return lookupAccessor(".", stack, assigned) } + findVarScope := func(v string) *loopScope { return lookupAccessor(v, stack, assigned) } + for i, line := range lines { for _, loc := range templateActionRe.FindAllStringSubmatchIndex(line, -1) { inner := strings.TrimSpace(line[loc[2]:loc[3]]) @@ -256,17 +374,55 @@ func (r *OpenAPIValuesQuoteRule) checkArrayRanges( case "else": continue case "if": - stack = append(stack, arrayScope{rebindDot: false}) + stack = append(stack, loopScope{rebindDot: false}) + continue + case "define", "block": + stack = append(stack, loopScope{rebindDot: true}) continue - case "with", "define", "block": - stack = append(stack, arrayScope{rebindDot: true}) + case "with": + stack = append(stack, resolveScope("with", inner, valuesKey, risky, stack, assigned, r)) continue case "range": - stack = append(stack, newRangeScope(inner, prefix, risky, r)) + stack = append(stack, resolveScope("range", inner, valuesKey, risky, stack, assigned, r)) continue } - tracked := emissionScope(leadingCommand(inner), innermostDotScope, findVarScope) + // Variable assignment (`{{ $y := … }}`) produces no output; it binds $y to + // the risk of its right-hand side so later `{{ $y }}` / `{{ $y.field }}` is + // still checked (aliasing). + if name, rhs, ok := parseAssignment(inner); ok { + if desc, ok := resolveAssigned(name, rhs, valuesKey, risky, stack, assigned, r); ok { + assigned[name] = desc + } else { + delete(assigned, name) + } + + continue + } + + // Cross-template flow: a risky value passed to a module template that renders + // it unquoted (`{{ include "mymod.env" .Values.mod.config }}`). + if name, arg, ok := parseInclude(inner); ok { + r.checkInclude(relPath, i+1, inner, name, arg, valuesKey, risky, stack, assigned, defineEmits, errorList) + continue + } + + // Passthrough function with a risky argument (`{{ printf "%s" .Values.mod.foo }}`, + // `{{ upper $v }}`, `{{ index .Values.mod.list 0 }}`) — the risky value flows to + // the unquoted output. + if lead := leadingCommand(inner); isPassthrough(lead) { + if path, ok := passthroughRisky(inner, valuesKey, risky, innermostDotScope, findVarScope); ok && !inBlock[i] && !pipelineIsSafe(inner) && r.Enabled(path) { + if report, wrap := analyzeContext(line[:loc[0]], line[loc[1]:]); report { + errorList.WithFilePath(relPath).WithLineNumber(i+1). + Errorf("value '.Values.%s.%s' is an OpenAPI string without a validation pattern (pattern/enum/format) %s", + valuesKey, path, quoteAdvice(wrap)) + } + } + + continue + } + + tracked, sub := emissionScope(leadingCommand(inner), innermostDotScope, findVarScope) if tracked == nil { continue } @@ -275,56 +431,704 @@ func (r *OpenAPIValuesQuoteRule) checkArrayRanges( continue } - if standalone, quoted := analyzeContext(line[:loc[0]], line[loc[1]:]); !standalone || quoted { + report, wrap := analyzeContext(line[:loc[0]], line[loc[1]:]) + if !report { continue } - errorList.WithFilePath(relPath).WithLineNumber(i+1). - Errorf("array element from '.Values.%s.%s' is an OpenAPI string without a validation pattern (pattern/enum/format) and must be quoted in templates: use '| quote' or wrap the value in quotes", - valuesKey, tracked.arrPath) + r.reportScoped(relPath, i+1, valuesKey, tracked, sub, wrap, errorList) } } } -// newRangeScope builds the scope for a `range` action, marking it as a tracked risky -// array iteration when it ranges over a risky array path that is not excluded. -func newRangeScope(inner, prefix string, risky *riskyPaths, rule *OpenAPIValuesQuoteRule) arrayScope { - sc := arrayScope{rebindDot: true} +// reportScoped emits the finding for a scoped emission, worded for a scalar value, an +// array element, or a string sub-field of an array-of-objects element. +func (r *OpenAPIValuesQuoteRule) reportScoped( + relPath string, line int, valuesKey string, scope *loopScope, sub string, wrap bool, errorList *errors.LintRuleErrorsList, +) { + tail := "is an OpenAPI string without a validation pattern (pattern/enum/format) " + quoteAdvice(wrap) - coll, elemVar := parseRange(inner) + el := errorList.WithFilePath(relPath).WithLineNumber(line) + + switch { + case scope.kind == "array": + el.Errorf("array element from '.Values.%s.%s' %s", valuesKey, scope.valPath, tail) + case scope.kind == "object" && scope.elemArray: + el.Errorf("array element field '.Values.%s.%s[].%s' %s", valuesKey, scope.valPath, sub, tail) + case scope.kind == "object": + el.Errorf("value '.Values.%s.%s.%s' %s", valuesKey, scope.valPath, sub, tail) + default: + el.Errorf("value '.Values.%s.%s' %s", valuesKey, scope.valPath, tail) + } +} + +// resolveScope builds the scope for a `range` or `with` block. It resolves the block +// subject to the risky namespace it selects — either root-scoped (`.Values..

` / +// `$.Values..

`) or relative to an enclosing element (`$s.` / `.`, the +// key to range-in-range) — and classifies it. A `range` tracks a risky string array +// (kind "array") or array-of-objects (kind "object"); a `with` tracks a risky string +// scalar (kind "scalar"). Any other block returns an untracked, dot-rebinding scope. +func resolveScope(block, inner, valuesKey string, root *riskyPaths, stack []loopScope, assigned map[string]loopScope, rule *OpenAPIValuesQuoteRule) loopScope { + sc := loopScope{rebindDot: true} + + var subject, elemVar string + + if block == "range" { + coll, ev := parseRange(inner) + subject, elemVar = firstToken(coll), ev + } else { + subject = firstToken(strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(inner), "with"))) + elemVar = "." + } + + // A bare variable / dot subject (`range $x`, `with $s`, `range $c := $row`) resolves + // against the enclosing scope or an assigned variable rather than a `.Values` path. + if accessor, tail := splitVarPath(subject); tail == "" && accessor != "" { + v := lookupAccessor(accessor, stack, assigned) + if v == nil || v.kind == "" { + return sc + } + + if out, ok := scopeFromVar(block, v, elemVar); ok && rule.Enabled(out.valPath) { + return out + } - collCmd := firstToken(coll) - if !strings.HasPrefix(collCmd, prefix) { return sc } - arr := collCmd[len(prefix):] - if _, ok := risky.array[arr]; !ok || !rule.Enabled(arr) { + base, sub, disp, ok := resolveBase(subject, valuesKey, root, stack, assigned) + if !ok || !rule.Enabled(disp) { + return sc + } + + switch { + case block == "range" && has(base.array, sub): + sc.kind = "array" + case block == "range" && has(base.arrayArray, sub): + sc.kind = "strarray" + case block == "range" && has(base.object, sub): + sc.kind = "object" + sc.elemArray = true + sc.profile = base.object[sub] + case block == "with" && has(base.scalar, sub): + sc.kind = "scalar" + case block == "with": + // `with` over an object value binds dot to the object; its risky string + // sub-fields are then reachable as `{{ .field }}` (a single object, so + // sub-fields display as `db.host`, not `db[].host`). + prof := subProfile(base, sub) + if prof == nil { + return sc + } + + sc.kind = "object" + sc.profile = prof + default: return sc } sc.elemVar = elemVar - sc.arrPath = arr + sc.valPath = disp return sc } -// emissionScope returns the tracked range scope an emission of cmd refers to, or nil. -// A bare dot resolves to the innermost dot-rebinding scope; a `$var` resolves to the -// nearest enclosing scope that introduced it. -func emissionScope(cmd string, innermostDot func() *arrayScope, findVar func(string) *arrayScope) *arrayScope { +// subProfile returns the risk profile of the object at prefix within base, with keys +// re-based to be relative to it, or nil when nothing risky lives under it. It lets a +// `with` over a (non-array) object reuse the same sub-field machinery as an +// array-of-objects element. +func subProfile(base *riskyPaths, prefix string) *riskyPaths { + pfx := prefix + "." + out := newRiskyPaths() + + for k := range base.scalar { + if rel, ok := strings.CutPrefix(k, pfx); ok { + out.scalar[rel] = struct{}{} + } + } + + for k := range base.array { + if rel, ok := strings.CutPrefix(k, pfx); ok { + out.array[rel] = struct{}{} + } + } + + for k := range base.arrayArray { + if rel, ok := strings.CutPrefix(k, pfx); ok { + out.arrayArray[rel] = struct{}{} + } + } + + for k, v := range base.object { + if rel, ok := strings.CutPrefix(k, pfx); ok { + out.object[rel] = v + } + } + + if out.empty() { + return nil + } + + return out +} + +// scopeFromVar builds a range/with scope from a variable's tracked risk. A `with` +// tracks an object variable (`{{ with $s }}{{ .host }}`); a `range` tracks a variable +// that holds — or whose element is — a string array (array-of-arrays inner range, +// `{{ range $x := .Values.list }}` bound then `{{ range $x }}`) or an array of objects. +// The bool is false when the variable is not usefully rangeable/scopeable. +func scopeFromVar(block string, v *loopScope, elemVar string) (loopScope, bool) { + sc := loopScope{rebindDot: true} + + if block == "with" { + if v.kind == "object" { + sc.kind, sc.elemVar, sc.valPath, sc.elemArray, sc.profile = "object", ".", v.valPath, v.elemArray, v.profile + return sc, true + } + + return sc, false + } + + switch v.kind { + case "strarray": + sc.kind, sc.elemVar, sc.valPath = "array", elemVar, v.valPath+"[]" + case "arrayvalue": + sc.kind, sc.elemVar, sc.valPath = "array", elemVar, v.valPath + case "objarrayvalue": + sc.kind, sc.elemVar, sc.valPath, sc.elemArray, sc.profile = "object", elemVar, v.valPath, true, v.profile + default: + return sc, false + } + + return sc, true +} + +// resolveBase maps a block subject command to the risk namespace it lives in, the +// sub-key within that namespace, and the display path. Root references resolve against +// the module values root; `$var.` / `.` resolve against the risk profile of +// the enclosing array-of-objects element (enabling range-in-range). +func resolveBase(cmd, valuesKey string, root *riskyPaths, stack []loopScope, assigned map[string]loopScope) (*riskyPaths, string, string, bool) { + if p, ok := valuePath(cmd, valuesKey); ok { + return root, p, p, true + } + + accessor, sub := splitVarPath(cmd) + if accessor == "" || sub == "" { + return nil, "", "", false + } + + sc := lookupAccessor(accessor, stack, assigned) + if sc == nil || sc.kind != "object" || sc.profile == nil { + return nil, "", "", false + } + + sep := "." + if sc.elemArray { + sep = "[]." + } + + return sc.profile, sub, sc.valPath + sep + sub, true +} + +// lookupAccessor finds the scope an element accessor refers to: a bare dot resolves to +// the innermost dot-rebinding scope (which may be untracked — the caller checks), a +// `$var` to the nearest enclosing range/with element variable or, failing that, a +// variable bound by an assignment (`{{ $y := … }}`). +func lookupAccessor(accessor string, stack []loopScope, assigned map[string]loopScope) *loopScope { + for i := len(stack) - 1; i >= 0; i-- { + if accessor == "." && stack[i].rebindDot { + return &stack[i] + } + + if accessor != "." && stack[i].elemVar == accessor { + return &stack[i] + } + } + + if accessor != "." { + if sc, ok := assigned[accessor]; ok { + return &sc + } + } + + return nil +} + +// has reports whether key k is present in map m of any value type. +func has[V any](m map[string]V, k string) bool { + _, ok := m[k] + return ok +} + +// parseAssignment recognizes a template variable assignment `$name := expr` / +// `$name = expr` and returns the variable name (with `$`) and the right-hand side. +func parseAssignment(inner string) (string, string, bool) { + m := assignRe.FindStringSubmatch(strings.TrimSpace(inner)) + if m == nil { + return "", "", false + } + + rhs := strings.TrimSpace(m[3]) + if strings.HasPrefix(rhs, "=") { // `==` comparison, not an assignment + return "", "", false + } + + return m[1], rhs, true +} + +// resolveValueExpr resolves an expression to the risk of the VALUE it denotes: an +// alias of a risky variable (`$x`), a risky scalar (`.Values.mod.foo` / `$s.host`), or +// a risky object whose profile is returned (`.Values.mod.db`, an array-of-objects +// element, …). It is the shared basis for variable assignment and `include`/`template` +// argument analysis. The returned scope has no elemVar; the caller sets one if needed. +func resolveValueExpr(cmd, valuesKey string, root *riskyPaths, stack []loopScope, assigned map[string]loopScope) (loopScope, bool) { + if accessor, tail := splitVarPath(cmd); tail == "" && (accessor == "." || strings.HasPrefix(accessor, "$")) { + src := lookupAccessor(accessor, stack, assigned) + if src == nil || src.kind == "" { + return loopScope{}, false + } + + alias := *src + alias.elemVar = "" + alias.rebindDot = false + + return alias, true + } + + base, sub, disp, ok := resolveBase(cmd, valuesKey, root, stack, assigned) + if !ok { + return loopScope{}, false + } + switch { - case cmd == ".": - if s := innermostDot(); s != nil && s.elemVar == "." { - return s + case has(base.scalar, sub): + return loopScope{kind: "scalar", valPath: disp}, true + case has(base.array, sub): + return loopScope{kind: "arrayvalue", valPath: disp}, true + case has(base.object, sub): + return loopScope{kind: "objarrayvalue", valPath: disp, elemArray: true, profile: base.object[sub]}, true + } + + if prof := subProfile(base, sub); prof != nil { + return loopScope{kind: "object", valPath: disp, profile: prof}, true + } + + return loopScope{}, false +} + +// resolveAssigned computes the risk a variable takes on from an assignment's +// right-hand side. It returns ok=false when the right-hand side is not risky (or +// already made safe by a pipeline function), so the caller clears any previous binding. +func resolveAssigned(name, rhs, valuesKey string, root *riskyPaths, stack []loopScope, assigned map[string]loopScope, rule *OpenAPIValuesQuoteRule) (loopScope, bool) { + if pipelineIsSafe(rhs) { + return loopScope{}, false + } + + desc, ok := resolveValueExpr(leadingCommand(rhs), valuesKey, root, stack, assigned) + if !ok || !rule.Enabled(desc.valPath) { + return loopScope{}, false + } + + desc.elemVar = name + + return desc, true +} + +// defineInfo records what a `define` block renders from its parameter (dot): whether it +// emits the bare parameter unquoted, which parameter-relative scalar sub-fields it emits +// unquoted, which parameter-relative array sub-fields it ranges and emits elements of +// unquoted, and which templates it forwards its parameter (or a sub-field) to. +type defineInfo struct { + bareDot bool + subs map[string]struct{} + arraySubs map[string]struct{} + calls []defineCall +} + +type defineCall struct { + name string + sub string // "" = forwarded `.`, "a.b" = forwarded `.a.b` +} + +// defineEmit is the resolved (transitive) set of parameter-relative values a template +// renders unquoted. +type defineEmit struct { + bareDot bool + subs map[string]struct{} + arraySubs map[string]struct{} +} + +// defineFrame is one entry on the block stack while scanning a define body. +type defineFrame struct { + def *defineInfo + rebind bool + rangeSub string // set when this range iterates the parameter sub-field + elemVar string // the range element accessor ("." or "$x") +} + +// parseInclude recognizes an `include "name" arg` / `template "name" arg` action and +// returns the template name and the (possibly empty) argument expression. +func parseInclude(inner string) (string, string, bool) { + m := includeRe.FindStringSubmatch(strings.TrimSpace(inner)) + if m == nil { + return "", "", false + } + + return m[1], strings.TrimSpace(m[2]), true +} + +// collectDefines scans the given template contents for `define` blocks and records, per +// template name, the parameter-relative values it renders unquoted (see defineInfo). +// Only the module's own defines are seen; external ones (helm_lib, …) are absent and +// therefore never matched, which keeps cross-template findings free of false positives. +func collectDefines(contents []string) map[string]*defineInfo { + defs := map[string]*defineInfo{} + + for _, content := range contents { + lines := strings.Split(content, "\n") + inBlock := computeBlockScalarLines(lines) + + var stack []defineFrame + + for i, line := range lines { + for _, loc := range templateActionRe.FindAllStringSubmatchIndex(line, -1) { + inner := strings.TrimSpace(line[loc[2]:loc[3]]) + + switch firstToken(inner) { + case "end": + if len(stack) > 0 { + stack = stack[:len(stack)-1] + } + + continue + case "else": + continue + case "define": + stack = append(stack, defineFrame{def: defForName(defs, inner)}) + continue + case "if": + stack = append(stack, defineFrame{}) + continue + case "with", "block": + stack = append(stack, defineFrame{rebind: true}) + continue + case "range": + stack = append(stack, rangeDefineFrame(stack, inner)) + continue + } + + def, depth, innermost := defineContext(stack) + if def == nil || inBlock[i] { + continue + } + + switch { + case depth == 0: + recordDefineUsage(def, inner, line[:loc[0]], line[loc[1]:]) + case depth == 1 && innermost != nil && innermost.rangeSub != "": + recordDefineArrayElem(def, innermost, inner, line[:loc[0]], line[loc[1]:]) + } + } + } + } + + return defs +} + +// defineContext returns the innermost enclosing define, the dot-rebinding depth within +// it, and the innermost rebinding frame (for spotting parameter-subfield ranges). +func defineContext(stack []defineFrame) (*defineInfo, int, *defineFrame) { + for i := len(stack) - 1; i >= 0; i-- { + if stack[i].def == nil { + continue + } + + depth := 0 + + var innermost *defineFrame + + for j := i + 1; j < len(stack); j++ { + if stack[j].rebind { + depth++ + innermost = &stack[j] + } + } + + return stack[i].def, depth, innermost + } + + return nil, 0, nil +} + +// rangeDefineFrame builds the stack frame for a `range` inside a define, marking it when +// it iterates a parameter sub-field at the parameter level (`{{ range .items }}`). +func rangeDefineFrame(stack []defineFrame, inner string) defineFrame { + f := defineFrame{rebind: true} + + if def, depth, _ := defineContext(stack); def == nil || depth != 0 { + return f + } + + coll, elemVar := parseRange(inner) + if accessor, sub := splitVarPath(firstToken(coll)); accessor == "." && sub != "" { + f.rangeSub = sub + f.elemVar = elemVar + } + + return f +} + +// defForName returns (creating if needed) the defineInfo for the `define "name"` +// action, or nil when the name cannot be parsed. +func defForName(defs map[string]*defineInfo, inner string) *defineInfo { + m := defineNameRe.FindStringSubmatch(inner) + if m == nil { + return nil + } + + if info := defs[m[1]]; info != nil { + return info + } + + info := &defineInfo{subs: map[string]struct{}{}, arraySubs: map[string]struct{}{}} + defs[m[1]] = info + + return info +} + +// recordDefineUsage records a parameter-relative emission or a parameter-forwarding +// include for the enclosing define (at the parameter level). +func recordDefineUsage(def *defineInfo, inner, pre, post string) { + if pipelineIsSafe(inner) { + return + } + + if name, arg, ok := parseInclude(inner); ok { + if accessor, sub := splitVarPath(leadingCommand(arg)); accessor == "." { + def.calls = append(def.calls, defineCall{name: name, sub: sub}) + } + + return + } + + accessor, sub := splitVarPath(leadingCommand(inner)) + if accessor != "." { + return + } + + if report, _ := analyzeContext(pre, post); !report { + return + } + + if sub == "" { + def.bareDot = true + } else { + def.subs[sub] = struct{}{} + } +} + +// recordDefineArrayElem records that a define renders, unquoted, the elements of the +// parameter array sub-field its enclosing `range` iterates. +func recordDefineArrayElem(def *defineInfo, rf *defineFrame, inner, pre, post string) { + if pipelineIsSafe(inner) { + return + } + + accessor, sub := splitVarPath(leadingCommand(inner)) + if sub != "" || accessor != rf.elemVar { + return + } + + if report, _ := analyzeContext(pre, post); !report { + return + } + + def.arraySubs[rf.rangeSub] = struct{}{} +} + +// resolveDefineEmits resolves each define's own and transitively forwarded emissions +// into a flat parameter-relative emit set per template name. +func resolveDefineEmits(defs map[string]*defineInfo) map[string]defineEmit { + out := make(map[string]defineEmit, len(defs)) + + var resolve func(name string, seen map[string]bool) defineEmit + + resolve = func(name string, seen map[string]bool) defineEmit { + info := defs[name] + if info == nil || seen[name] { + return defineEmit{subs: map[string]struct{}{}, arraySubs: map[string]struct{}{}} + } + + seen[name] = true + defer delete(seen, name) + + res := defineEmit{bareDot: info.bareDot, subs: map[string]struct{}{}, arraySubs: map[string]struct{}{}} + for s := range info.subs { + res.subs[s] = struct{}{} + } + + for s := range info.arraySubs { + res.arraySubs[s] = struct{}{} + } + + for _, call := range info.calls { + sub := resolve(call.name, seen) + + if call.sub == "" { + res.bareDot = res.bareDot || sub.bareDot + mergeSet(res.subs, sub.subs, "") + mergeSet(res.arraySubs, sub.arraySubs, "") + + continue + } + + if sub.bareDot { + res.subs[call.sub] = struct{}{} + } + + mergeSet(res.subs, sub.subs, call.sub+".") + mergeSet(res.arraySubs, sub.arraySubs, call.sub+".") + } + + return res + } + + for name := range defs { + out[name] = resolve(name, map[string]bool{}) + } + + return out +} + +// mergeSet copies every key of src into dst, each prefixed by prefix. +func mergeSet(dst, src map[string]struct{}, prefix string) { + for s := range src { + dst[prefix+s] = struct{}{} + } +} + +// checkInclude flags a risky value that reaches a module template which renders it +// unquoted. It fires only when the argument resolves to a concrete risky value and the +// called template's body (seen in the module) emits it unquoted. +func (r *OpenAPIValuesQuoteRule) checkInclude( + relPath string, line int, inner, name, arg, valuesKey string, + root *riskyPaths, stack []loopScope, assigned map[string]loopScope, + emits map[string]defineEmit, errorList *errors.LintRuleErrorsList, +) { + emit, ok := emits[name] + if !ok || pipelineIsSafe(inner) { + return + } + + desc, ok := resolveValueExpr(leadingCommand(arg), valuesKey, root, stack, assigned) + if !ok { + return + } + + report := func(path string) { + if !r.Enabled(path) { + return + } + + errorList.WithFilePath(relPath).WithLineNumber(line). + Errorf("value '.Values.%s.%s' is an OpenAPI string without a validation pattern (pattern/enum/format) and is rendered unquoted by template %q; quote it there or before passing to the template", + valuesKey, path, name) + } + + switch { + case desc.kind == "scalar" && emit.bareDot: + report(desc.valPath) + case desc.kind == "object" && desc.profile != nil: + sep := "." + if desc.elemArray { + sep = "[]." + } + + for sub := range emit.subs { + if has(desc.profile.scalar, sub) { + report(desc.valPath + sep + sub) + } + } + + for sub := range emit.arraySubs { + if has(desc.profile.array, sub) { + report(desc.valPath + sep + sub) + } + } + } +} + +// emissionScope resolves an emission command to the tracked scope it belongs to and, +// for an array-of-objects element, the risky sub-field it accesses. It returns +// (nil, "") when the command is not a tracked risky emission. +// +// The command is split into the element accessor and an optional sub-path: `.`/`$x` +// (the element itself) or `.field`/`$x.field.sub` (a sub-field). A bare-dot accessor +// resolves to the innermost dot-rebinding scope; a `$var` accessor to the nearest +// enclosing scope that introduced it. +func emissionScope(cmd string, innermostDot func() *loopScope, findVar func(string) *loopScope) (*loopScope, string) { + accessor, sub := splitVarPath(cmd) + if accessor == "" { + return nil, "" + } + + var s *loopScope + if accessor == "." { + s = innermostDot() + } else { + s = findVar(accessor) + } + + if s == nil || s.kind == "" || s.elemVar != accessor { + return nil, "" + } + + switch s.kind { + case "array", "scalar": + if sub == "" { + return s, "" } + case "object": + if s.profile != nil && has(s.profile.scalar, sub) { + return s, sub + } + } + + return nil, "" +} + +// splitVarPath splits an emission command into the loop-element accessor and the +// sub-path accessed on it: "." -> (".", ""), ".host" -> (".", "host"), +// "$s" -> ("$s", ""), "$s.spec.name" -> ("$s", "spec.name"). Any other command +// (a function, a `.Values...` reference, …) yields ("", ""). +func splitVarPath(cmd string) (string, string) { + switch { + case cmd == ".": + return ".", "" case strings.HasPrefix(cmd, "$"): - if s := findVar(cmd); s != nil && s.elemVar == cmd { - return s + if accessor, sub, found := strings.Cut(cmd, "."); found { + return accessor, sub } + + return cmd, "" + case strings.HasPrefix(cmd, "."): + return ".", cmd[1:] + default: + return "", "" } +} - return nil +// valuePath returns the module value path a command refers to via `.Values..

` +// or the root-scoped `$.Values..

` (valid inside range/with), and whether it +// matched. +func valuePath(cmd, valuesKey string) (string, bool) { + for _, prefix := range []string{".Values." + valuesKey + ".", "$.Values." + valuesKey + "."} { + if rest, ok := strings.CutPrefix(cmd, prefix); ok { + return rest, true + } + } + + return "", false } // parseRange splits a `range ...` action into the collection expression and the loop @@ -343,13 +1147,44 @@ func parseRange(inner string) (string, string) { // leadingCommand returns the first token of the first stage of a template pipeline — // the command that produces the value (`.Values.x`, `if`, `range`, `$v`, `quote`, …). +// A first stage fully wrapped in parentheses is unwrapped first, so `(.Values.x)` +// reads as `.Values.x`. func leadingCommand(inner string) string { stages := splitPipeline(inner) if len(stages) == 0 { return "" } - return firstToken(stages[0]) + s := strings.TrimSpace(stages[0]) + for isParenWrapped(s) { + s = strings.TrimSpace(s[1 : len(s)-1]) + } + + return firstToken(s) +} + +// isParenWrapped reports whether s is a single parenthesised group, i.e. its first +// `(` matches its final `)`. +func isParenWrapped(s string) bool { + if len(s) < 2 || s[0] != '(' || s[len(s)-1] != ')' { + return false + } + + depth := 0 + + for i := 0; i < len(s); i++ { + switch s[i] { + case '(': + depth++ + case ')': + depth-- + if depth == 0 { + return i == len(s)-1 + } + } + } + + return false } // pipelineIsSafe reports whether any function stage of the pipeline is known to emit @@ -409,45 +1244,44 @@ func firstToken(s string) string { return s } -// analyzeContext inspects the text around an action to decide whether the action is -// the entire YAML value (standalone) and, if so, whether it is wrapped in matching -// quotes. Non-standalone actions (embedded in a larger scalar, flow collections, …) -// return standalone=false and are skipped to avoid false positives. +// analyzeContext inspects the text around an action to decide whether the value must +// be reported and, if so, how. report is false when the value is wrapped in matching +// quotes (safe) or the context is ambiguous (a lone quote on one side). wrap is true +// when the action is embedded in a larger unquoted scalar (`host: pre-{{ … }}`), where +// the fix is to quote the whole value rather than pipe it through `quote`. func analyzeContext(pre, post string) (bool, bool) { - post = strings.TrimRight(stripTrailingComment(post), " \t") - - var closeQuote byte - - if rest := strings.TrimLeft(post, " \t"); rest != "" { - if rest[0] != '"' && rest[0] != '\'' { - return false, false - } - - closeQuote = rest[0] - - if strings.TrimSpace(rest[1:]) != "" { - return false, false - } - } - - m := standaloneOpenRe.FindStringSubmatch(pre) + m := valuePreRe.FindStringSubmatch(pre) if m == nil { return false, false } + valuePre := strings.TrimLeft(m[3], " \t") + var openQuote byte - if m[3] != "" { - openQuote = m[3][0] + if len(valuePre) > 0 && (valuePre[0] == '"' || valuePre[0] == '\'') { + openQuote = valuePre[0] + valuePre = valuePre[1:] } + tail := strings.TrimRight(stripTrailingComment(post), " \t") + + var closeQuote byte + if len(tail) > 0 && (tail[len(tail)-1] == '"' || tail[len(tail)-1] == '\'') { + closeQuote = tail[len(tail)-1] + tail = tail[:len(tail)-1] + } + + embedded := strings.TrimSpace(valuePre) != "" || strings.TrimSpace(tail) != "" + switch { - case openQuote == 0 && closeQuote == 0: - return true, false case openQuote != 0 && openQuote == closeQuote: - return true, true + return false, false // wrapped in matching quotes -> safe + case openQuote != 0 || closeQuote != 0: + return false, false // a lone quote on one side -> ambiguous, skip + case embedded: + return true, true // embedded in a larger unquoted scalar -> wrap the whole value default: - // A quote on only one side is ambiguous; skip conservatively. - return false, false + return true, false // standalone, unquoted -> pipe through quote } } @@ -533,18 +1367,57 @@ func (w *schemaWalker) walk(path string, schema map[string]any, depth int) { if types["array"] && path != "" { if items, ok := schema["items"].(map[string]any); ok { - resolved := w.resolveRef(items) - if schemaTypes(resolved)["string"] && !hasValidation(resolved) { - w.out.array[path] = struct{}{} - } + w.walkItems(path, w.resolveRef(items), depth) } } + // A map's values (additionalProperties) are iterated by `range` exactly like array + // elements — the loop value is the map value — so they are classified the same way. + if ap, ok := schema["additionalProperties"].(map[string]any); ok && path != "" { + w.walkItems(path, w.resolveRef(ap), depth) + } + if types["string"] && path != "" && !hasValidation(schema) { w.out.scalar[path] = struct{}{} } } +// walkItems classifies the element/value schema of an array or map: a plain +// unvalidated string registers the array path; an array of unvalidated strings +// registers an array-of-arrays path (checked through range-in-range); an object +// registers the element's recursive risk profile so `range $x := arr` / +// `{{ $x.field }}` (and nested ranges) can be checked. +func (w *schemaWalker) walkItems(path string, items map[string]any, depth int) { + itemTypes := schemaTypes(items) + + if itemTypes["string"] && !hasValidation(items) { + w.out.array[path] = struct{}{} + return + } + + if itemTypes["array"] { + if inner, ok := items["items"].(map[string]any); ok { + resolved := w.resolveRef(inner) + if schemaTypes(resolved)["string"] && !hasValidation(resolved) { + w.out.arrayArray[path] = struct{}{} + } + } + + return + } + + if _, ok := items["properties"].(map[string]any); !ok { + return + } + + sub := newRiskyPaths() + (&schemaWalker{root: w.root, out: sub}).walk("", items, depth+1) + + if !sub.empty() { + w.out.object[path] = sub + } +} + // resolveRef resolves a local `$ref` ("#/...") chain, merging sibling keys of the // referring schema over the target. A non-local or unresolvable ref is returned as-is. func (w *schemaWalker) resolveRef(schema map[string]any) map[string]any { diff --git a/pkg/linters/templates/rules/openapi_values_quote_test.go b/pkg/linters/templates/rules/openapi_values_quote_test.go index 61ff2f0a..aa358c33 100644 --- a/pkg/linters/templates/rules/openapi_values_quote_test.go +++ b/pkg/linters/templates/rules/openapi_values_quote_test.go @@ -281,7 +281,7 @@ properties: wantCount: 0, }, { - name: "value embedded in a larger scalar is not flagged", + name: "value embedded in a larger unquoted scalar is flagged (wrap advice)", valuesSchema: `type: object properties: foo: @@ -290,6 +290,19 @@ properties: files: map[string]string{ "templates/cm.yaml": "data:\n foo: prefix-{{ .Values." + valuesKey + ".foo }}\n", }, + wantCount: 1, + wantContains: []string{"wrap the whole value in quotes"}, + }, + { + name: "value embedded in a quoted scalar is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: \"prefix-{{ .Values." + valuesKey + ".foo }}\"\n", + }, wantCount: 0, }, { @@ -354,6 +367,1085 @@ properties: wantCount: 1, wantContains: []string{"foo"}, }, + { + name: "value piped through squote is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ .Values." + valuesKey + ".foo | squote }}\n", + }, + wantCount: 0, + }, + { + name: "value piped through toJson is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ .Values." + valuesKey + ".foo | toJson }}\n", + }, + wantCount: 0, + }, + { + name: "value piped through toYaml is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ .Values." + valuesKey + ".foo | toYaml }}\n", + }, + wantCount: 0, + }, + { + name: "value piped through an unrelated function is flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ .Values." + valuesKey + ".foo | trim }}\n", + }, + wantCount: 1, + wantContains: []string{"foo"}, + }, + { + name: "only maxLength does not exempt (still flagged)", + valuesSchema: `type: object +properties: + foo: + type: string + maxLength: 10 +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ .Values." + valuesKey + ".foo }}\n", + }, + wantCount: 1, + wantContains: []string{"foo"}, + }, + { + name: "empty enum does not exempt (still flagged)", + valuesSchema: `type: object +properties: + foo: + type: string + enum: [] +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ .Values." + valuesKey + ".foo }}\n", + }, + wantCount: 1, + wantContains: []string{"foo"}, + }, + { + name: "empty pattern does not exempt (still flagged)", + valuesSchema: `type: object +properties: + foo: + type: string + pattern: '' +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ .Values." + valuesKey + ".foo }}\n", + }, + wantCount: 1, + wantContains: []string{"foo"}, + }, + { + name: "string branch of anyOf is flagged", + valuesSchema: `type: object +properties: + foo: + anyOf: + - type: string + - type: integer +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ .Values." + valuesKey + ".foo }}\n", + }, + wantCount: 1, + wantContains: []string{"foo"}, + }, + { + name: "deeply nested string path is flagged with full path", + valuesSchema: `type: object +properties: + a: + type: object + properties: + b: + type: object + properties: + c: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n c: {{ .Values." + valuesKey + ".a.b.c }}\n", + }, + wantCount: 1, + wantContains: []string{"a.b.c"}, + }, + { + name: "multiple unquoted strings in one file are all flagged", + valuesSchema: `type: object +properties: + foo: + type: string + bar: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ .Values." + valuesKey + ".foo }}\n bar: {{ .Values." + valuesKey + ".bar }}\n", + }, + wantCount: 2, + wantContains: []string{"foo", "bar"}, + }, + { + name: "unquoted scalar via with is flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "{{- with .Values." + valuesKey + ".foo }}\ndata:\n x: {{ . }}\n{{- end }}\n", + }, + wantCount: 1, + wantContains: []string{"foo"}, + }, + { + name: "quoted scalar via with is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "{{- with .Values." + valuesKey + ".foo }}\ndata:\n x: {{ . | quote }}\n{{- end }}\n", + }, + wantCount: 0, + }, + { + name: "root-scoped $.Values scalar is flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ $.Values." + valuesKey + ".foo }}\n", + }, + wantCount: 1, + wantContains: []string{"foo"}, + }, + { + name: "unquoted array element via index+element range is flagged", + valuesSchema: `type: object +properties: + list: + type: array + items: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "items:\n{{- range $i, $e := .Values." + valuesKey + ".list }}\n - {{ $e }}\n{{- end }}\n", + }, + wantCount: 1, + wantContains: []string{"array element", "list"}, + }, + { + name: "value passed to printf %s is flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ printf \"%s-x\" .Values." + valuesKey + ".foo }}\n", + }, + wantCount: 1, + wantContains: []string{"value '.Values." + valuesKey + ".foo'"}, + }, + { + name: "value passed to printf %q is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ printf \"%q\" .Values." + valuesKey + ".foo }}\n", + }, + wantCount: 0, + }, + { + name: "value passed to an unknown function is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ include \"mymod.helper\" .Values." + valuesKey + ".foo }}\n", + }, + wantCount: 0, + }, + { + name: "value in a parenthesised group is flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ (.Values." + valuesKey + ".foo) }}\n", + }, + wantCount: 1, + wantContains: []string{"value '.Values." + valuesKey + ".foo'"}, + }, + { + name: "risky loop variable passed to a passthrough function is flagged", + valuesSchema: `type: object +properties: + list: + type: array + items: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "items:\n{{- range $v := .Values." + valuesKey + ".list }}\n - {{ upper $v }}\n{{- end }}\n", + }, + wantCount: 1, + wantContains: []string{"list"}, + }, + { + name: "risky object-subfield variable passed to printf is flagged", + valuesSchema: `type: object +properties: + servers: + type: array + items: + type: object + properties: + host: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "servers:\n{{- range $s := .Values." + valuesKey + ".servers }}\n host: {{ printf \"%s\" $s.host }}\n{{- end }}\n", + }, + wantCount: 1, + wantContains: []string{"servers[].host"}, + }, + { + name: "quoted passthrough of a variable is not flagged", + valuesSchema: `type: object +properties: + list: + type: array + items: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "items:\n{{- range $v := .Values." + valuesKey + ".list }}\n - {{ upper $v | quote }}\n{{- end }}\n", + }, + wantCount: 0, + }, + { + name: "index into a risky array is flagged", + valuesSchema: `type: object +properties: + list: + type: array + items: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n first: {{ index .Values." + valuesKey + ".list 0 }}\n", + }, + wantCount: 1, + wantContains: []string{"list"}, + }, + { + name: "additionalProperties map value is not matched", + valuesSchema: `type: object +properties: + labels: + type: object + additionalProperties: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n x: {{ .Values." + valuesKey + ".labels.somekey }}\n", + }, + wantCount: 0, + }, + { + name: "template in a .tpl file is scanned", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/_helpers.tpl": "x: {{ .Values." + valuesKey + ".foo }}\n", + }, + wantCount: 1, + wantContains: []string{"foo"}, + }, + { + name: "quoted array element via named range variable is not flagged", + valuesSchema: `type: object +properties: + list: + type: array + items: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "items:\n{{- range $x := .Values." + valuesKey + ".list }}\n - {{ $x | quote }}\n{{- end }}\n", + }, + wantCount: 0, + }, + { + name: "array element via named range variable wrapped in quotes is not flagged", + valuesSchema: `type: object +properties: + list: + type: array + items: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "items:\n{{- range $x := .Values." + valuesKey + ".list }}\n - \"{{ $x }}\"\n{{- end }}\n", + }, + wantCount: 0, + }, + { + name: "array element via index+element range with squote is not flagged", + valuesSchema: `type: object +properties: + list: + type: array + items: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "items:\n{{- range $i, $x := .Values." + valuesKey + ".list }}\n - {{ $x | squote }}\n{{- end }}\n", + }, + wantCount: 0, + }, + { + name: "unquoted named range variable inside a nested if is flagged", + valuesSchema: `type: object +properties: + list: + type: array + items: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "items:\n{{- range $x := .Values." + valuesKey + ".list }}\n{{- if $x }}\n - {{ $x }}\n{{- end }}\n{{- end }}\n", + }, + wantCount: 1, + wantContains: []string{"array element", "list"}, + }, + { + name: "root-scoped $.Values scalar quoted inside range is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string + list: + type: array + items: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "{{- range .Values." + valuesKey + ".list }}\ndata:\n x: {{ $.Values." + valuesKey + ".foo | quote }}\n{{- end }}\n", + }, + wantCount: 0, + }, + { + name: "unquoted string subfield of an object array element is flagged", + valuesSchema: `type: object +properties: + servers: + type: array + items: + type: object + properties: + host: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "servers:\n{{- range $s := .Values." + valuesKey + ".servers }}\n host: {{ $s.host }}\n{{- end }}\n", + }, + wantCount: 1, + wantContains: []string{"array element field", "servers[].host"}, + }, + { + name: "quoted string subfield of an object array element is not flagged", + valuesSchema: `type: object +properties: + servers: + type: array + items: + type: object + properties: + host: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "servers:\n{{- range $s := .Values." + valuesKey + ".servers }}\n host: {{ $s.host | quote }}\n{{- end }}\n", + }, + wantCount: 0, + }, + { + name: "string subfield wrapped in quotes is not flagged", + valuesSchema: `type: object +properties: + servers: + type: array + items: + type: object + properties: + host: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "servers:\n{{- range $s := .Values." + valuesKey + ".servers }}\n host: \"{{ $s.host }}\"\n{{- end }}\n", + }, + wantCount: 0, + }, + { + name: "unquoted subfield via dot-binding range is flagged", + valuesSchema: `type: object +properties: + servers: + type: array + items: + type: object + properties: + host: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "servers:\n{{- range .Values." + valuesKey + ".servers }}\n host: {{ .host }}\n{{- end }}\n", + }, + wantCount: 1, + wantContains: []string{"servers[].host"}, + }, + { + name: "unquoted deeply-nested subfield is flagged with full sub-path", + valuesSchema: `type: object +properties: + servers: + type: array + items: + type: object + properties: + spec: + type: object + properties: + name: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "servers:\n{{- range $s := .Values." + valuesKey + ".servers }}\n name: {{ $s.spec.name }}\n{{- end }}\n", + }, + wantCount: 1, + wantContains: []string{"servers[].spec.name"}, + }, + { + name: "subfield constrained by a pattern is not flagged", + valuesSchema: `type: object +properties: + servers: + type: array + items: + type: object + properties: + host: + type: string + pattern: '^[a-z]+$' +`, + files: map[string]string{ + "templates/cm.yaml": "servers:\n{{- range $s := .Values." + valuesKey + ".servers }}\n host: {{ $s.host }}\n{{- end }}\n", + }, + wantCount: 0, + }, + { + name: "non-string subfield of an object array element is not flagged", + valuesSchema: `type: object +properties: + servers: + type: array + items: + type: object + properties: + port: + type: integer +`, + files: map[string]string{ + "templates/cm.yaml": "servers:\n{{- range $s := .Values." + valuesKey + ".servers }}\n port: {{ $s.port }}\n{{- end }}\n", + }, + wantCount: 0, + }, + { + name: "mixed object array: only unquoted risky subfields are flagged", + valuesSchema: `type: object +properties: + servers: + type: array + items: + type: object + properties: + host: + type: string + name: + type: string + env: + type: string + enum: ["prod", "dev"] +`, + files: map[string]string{ + "templates/cm.yaml": "servers:\n{{- range $s := .Values." + valuesKey + ".servers }}\n" + + " host: {{ $s.host }}\n" + // FLAG + " name: {{ $s.name | quote }}\n" + // safe (quote) + " env: {{ $s.env }}\n" + // safe (enum) + "{{- end }}\n", + }, + wantCount: 1, + wantContains: []string{"servers[].host"}, + }, + { + name: "unquoted element of a string-array subfield via nested range is flagged", + valuesSchema: `type: object +properties: + servers: + type: array + items: + type: object + properties: + tags: + type: array + items: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "servers:\n" + + "{{- range $s := .Values." + valuesKey + ".servers }}\n" + + "{{- range $t := $s.tags }}\n" + + " - {{ $t }}\n" + + "{{- end }}\n" + + "{{- end }}\n", + }, + wantCount: 1, + wantContains: []string{"array element from", "servers[].tags"}, + }, + { + name: "quoted element of a string-array subfield via nested range is not flagged", + valuesSchema: `type: object +properties: + servers: + type: array + items: + type: object + properties: + tags: + type: array + items: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "servers:\n" + + "{{- range $s := .Values." + valuesKey + ".servers }}\n" + + "{{- range $t := $s.tags }}\n" + + " - {{ $t | quote }}\n" + + "{{- end }}\n" + + "{{- end }}\n", + }, + wantCount: 0, + }, + { + name: "nested range with dot-binding is flagged", + valuesSchema: `type: object +properties: + servers: + type: array + items: + type: object + properties: + tags: + type: array + items: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "servers:\n" + + "{{- range .Values." + valuesKey + ".servers }}\n" + + "{{- range .tags }}\n" + + " - {{ . }}\n" + + "{{- end }}\n" + + "{{- end }}\n", + }, + wantCount: 1, + wantContains: []string{"servers[].tags"}, + }, + { + name: "string-array subfield with item pattern via nested range is not flagged", + valuesSchema: `type: object +properties: + servers: + type: array + items: + type: object + properties: + tags: + type: array + items: + type: string + pattern: '^[a-z]+$' +`, + files: map[string]string{ + "templates/cm.yaml": "servers:\n" + + "{{- range $s := .Values." + valuesKey + ".servers }}\n" + + "{{- range $t := $s.tags }}\n" + + " - {{ $t }}\n" + + "{{- end }}\n" + + "{{- end }}\n", + }, + wantCount: 0, + }, + { + name: "unquoted subfield of an object array nested in an object array is flagged", + valuesSchema: `type: object +properties: + servers: + type: array + items: + type: object + properties: + endpoints: + type: array + items: + type: object + properties: + url: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "servers:\n" + + "{{- range $s := .Values." + valuesKey + ".servers }}\n" + + "{{- range $e := $s.endpoints }}\n" + + " url: {{ $e.url }}\n" + + "{{- end }}\n" + + "{{- end }}\n", + }, + wantCount: 1, + wantContains: []string{"array element field", "servers[].endpoints[].url"}, + }, + + // --- (2) with over a single object --- + { + name: "unquoted subfield via with over a single object is flagged", + valuesSchema: `type: object +properties: + db: + type: object + properties: + host: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "{{- with .Values." + valuesKey + ".db }}\nhost: {{ .host }}\n{{- end }}\n", + }, + wantCount: 1, + wantContains: []string{"value '.Values." + valuesKey + ".db.host'"}, + }, + { + name: "quoted subfield via with over a single object is not flagged", + valuesSchema: `type: object +properties: + db: + type: object + properties: + host: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "{{- with .Values." + valuesKey + ".db }}\nhost: {{ .host | quote }}\n{{- end }}\n", + }, + wantCount: 0, + }, + + // --- (5) additionalProperties (maps) --- + { + name: "unquoted map value via range is flagged", + valuesSchema: `type: object +properties: + someMap: + type: object + additionalProperties: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n{{- range $k, $v := .Values." + valuesKey + ".someMap }}\n {{ $k }}: {{ $v }}\n{{- end }}\n", + }, + wantCount: 1, + wantContains: []string{"array element from", "someMap"}, + }, + { + name: "quoted map value via range is not flagged", + valuesSchema: `type: object +properties: + someMap: + type: object + additionalProperties: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n{{- range $k, $v := .Values." + valuesKey + ".someMap }}\n {{ $k }}: {{ $v | quote }}\n{{- end }}\n", + }, + wantCount: 0, + }, + { + name: "map value with item pattern is not flagged", + valuesSchema: `type: object +properties: + someMap: + type: object + additionalProperties: + type: string + pattern: '^[a-z]+$' +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n{{- range $k, $v := .Values." + valuesKey + ".someMap }}\n {{ $k }}: {{ $v }}\n{{- end }}\n", + }, + wantCount: 0, + }, + { + name: "unquoted subfield of a map-of-objects value is flagged", + valuesSchema: `type: object +properties: + someMap: + type: object + additionalProperties: + type: object + properties: + host: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n{{- range $k, $s := .Values." + valuesKey + ".someMap }}\n {{ $k }}:\n host: {{ $s.host }}\n{{- end }}\n", + }, + wantCount: 1, + wantContains: []string{"someMap[].host"}, + }, + + // --- (6) array of arrays of strings --- + { + name: "unquoted element of an array-of-arrays via nested range is flagged", + valuesSchema: `type: object +properties: + matrix: + type: array + items: + type: array + items: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "rows:\n" + + "{{- range $row := .Values." + valuesKey + ".matrix }}\n" + + "{{- range $c := $row }}\n" + + " - {{ $c }}\n" + + "{{- end }}\n" + + "{{- end }}\n", + }, + wantCount: 1, + wantContains: []string{"array element from", "matrix"}, + }, + { + name: "quoted element of an array-of-arrays is not flagged", + valuesSchema: `type: object +properties: + matrix: + type: array + items: + type: array + items: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "rows:\n" + + "{{- range $row := .Values." + valuesKey + ".matrix }}\n" + + "{{- range $c := $row }}\n" + + " - {{ $c | quote }}\n" + + "{{- end }}\n" + + "{{- end }}\n", + }, + wantCount: 0, + }, + + // --- (1) variable aliasing --- + { + name: "aliased scalar variable emitted unquoted is flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "{{ $y := .Values." + valuesKey + ".foo }}\ndata:\n x: {{ $y }}\n", + }, + wantCount: 1, + wantContains: []string{"value '.Values." + valuesKey + ".foo'"}, + }, + { + name: "aliased scalar variable emitted quoted is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "{{ $y := .Values." + valuesKey + ".foo }}\ndata:\n x: {{ $y | quote }}\n", + }, + wantCount: 0, + }, + { + name: "assignment of a safe-piped value is not tracked", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "{{ $y := .Values." + valuesKey + ".foo | quote }}\ndata:\n x: {{ $y }}\n", + }, + wantCount: 0, + }, + { + name: "alias of a loop element variable is flagged", + valuesSchema: `type: object +properties: + list: + type: array + items: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "items:\n{{- range $x := .Values." + valuesKey + ".list }}\n{{ $y := $x }}\n - {{ $y }}\n{{- end }}\n", + }, + wantCount: 1, + wantContains: []string{"array element from", "list"}, + }, + { + name: "aliased object variable subfield emitted unquoted is flagged", + valuesSchema: `type: object +properties: + db: + type: object + properties: + host: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "{{ $y := .Values." + valuesKey + ".db }}\ndata:\n host: {{ $y.host }}\n", + }, + wantCount: 1, + wantContains: []string{"value '.Values." + valuesKey + ".db.host'"}, + }, + { + name: "with over an object variable is flagged", + valuesSchema: `type: object +properties: + db: + type: object + properties: + host: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "{{ $s := .Values." + valuesKey + ".db }}\n{{- with $s }}\nhost: {{ .host }}\n{{- end }}\n", + }, + wantCount: 1, + wantContains: []string{"value '.Values." + valuesKey + ".db.host'"}, + }, + { + name: "array assigned to a variable then ranged is flagged", + valuesSchema: `type: object +properties: + list: + type: array + items: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "{{ $x := .Values." + valuesKey + ".list }}\nitems:\n{{- range $x }}\n - {{ . }}\n{{- end }}\n", + }, + wantCount: 1, + wantContains: []string{"array element from", "list"}, + }, + { + name: "array assigned to a variable then ranged with quote is not flagged", + valuesSchema: `type: object +properties: + list: + type: array + items: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "{{ $x := .Values." + valuesKey + ".list }}\nitems:\n{{- range $x }}\n - {{ . | quote }}\n{{- end }}\n", + }, + wantCount: 0, + }, + + // --- (4) cross-template flow --- + { + name: "risky scalar passed to a define that emits it unquoted is flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/_helpers.tpl": "{{- define \"mymod.raw\" -}}\n{{ . }}\n{{- end -}}\n", + "templates/cm.yaml": "data:\n x: {{ include \"mymod.raw\" .Values." + valuesKey + ".foo }}\n", + }, + wantCount: 1, + wantContains: []string{"value '.Values." + valuesKey + ".foo'", "mymod.raw"}, + }, + { + name: "risky object subfield emitted unquoted by a define is flagged", + valuesSchema: `type: object +properties: + db: + type: object + properties: + host: + type: string +`, + files: map[string]string{ + "templates/_helpers.tpl": "{{- define \"mymod.conn\" -}}\nhost: {{ .host }}\n{{- end -}}\n", + "templates/cm.yaml": "{{ include \"mymod.conn\" .Values." + valuesKey + ".db }}\n", + }, + wantCount: 1, + wantContains: []string{"value '.Values." + valuesKey + ".db.host'", "mymod.conn"}, + }, + { + name: "quoted emission in a define is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/_helpers.tpl": "{{- define \"mymod.raw\" -}}\n{{ . | quote }}\n{{- end -}}\n", + "templates/cm.yaml": "data:\n x: {{ include \"mymod.raw\" .Values." + valuesKey + ".foo }}\n", + }, + wantCount: 0, + }, + { + name: "include output quoted as a whole is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/_helpers.tpl": "{{- define \"mymod.raw\" -}}\n{{ . }}\n{{- end -}}\n", + "templates/cm.yaml": "data:\n x: {{ include \"mymod.raw\" .Values." + valuesKey + ".foo | quote }}\n", + }, + wantCount: 0, + }, + { + name: "include of an unknown external template is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n x: {{ include \"helm_lib.foo\" .Values." + valuesKey + ".foo }}\n", + }, + wantCount: 0, + }, + { + name: "transitive define forwarding is flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/_helpers.tpl": "{{- define \"a\" -}}\n{{ include \"b\" . }}\n{{- end -}}\n" + + "{{- define \"b\" -}}\n{{ . }}\n{{- end -}}\n", + "templates/cm.yaml": "data:\n x: {{ include \"a\" .Values." + valuesKey + ".foo }}\n", + }, + wantCount: 1, + wantContains: []string{"value '.Values." + valuesKey + ".foo'"}, + }, + { + name: "constrained scalar passed to an emitting define is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string + pattern: '^[a-z]+$' +`, + files: map[string]string{ + "templates/_helpers.tpl": "{{- define \"mymod.raw\" -}}\n{{ . }}\n{{- end -}}\n", + "templates/cm.yaml": "data:\n x: {{ include \"mymod.raw\" .Values." + valuesKey + ".foo }}\n", + }, + wantCount: 0, + }, + { + name: "array subfield ranged and emitted unquoted by a define is flagged", + valuesSchema: `type: object +properties: + config: + type: object + properties: + items: + type: array + items: + type: string +`, + files: map[string]string{ + "templates/_helpers.tpl": "{{- define \"mymod.args\" -}}\n{{- range .items }}\n- {{ . }}\n{{- end }}\n{{- end -}}\n", + "templates/cm.yaml": "args:\n{{ include \"mymod.args\" .Values." + valuesKey + ".config }}\n", + }, + wantCount: 1, + wantContains: []string{"config.items", "mymod.args"}, + }, + { + name: "array subfield ranged and quoted by a define is not flagged", + valuesSchema: `type: object +properties: + config: + type: object + properties: + items: + type: array + items: + type: string +`, + files: map[string]string{ + "templates/_helpers.tpl": "{{- define \"mymod.args\" -}}\n{{- range .items }}\n- {{ . | quote }}\n{{- end }}\n{{- end -}}\n", + "templates/cm.yaml": "args:\n{{ include \"mymod.args\" .Values." + valuesKey + ".config }}\n", + }, + wantCount: 0, + }, } for _, tt := range tests { diff --git a/test/e2e/testdata/templates/openapi-values-quote-forms/expected.yaml b/test/e2e/testdata/templates/openapi-values-quote-forms/expected.yaml new file mode 100644 index 00000000..b4524899 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-forms/expected.yaml @@ -0,0 +1,113 @@ +description: > + Covers every openapi-values-quote template form: passthrough over a variable, index, + with over an object variable, array-var assignment, aliasing, array-of-arrays, + map-of-objects, cross-template (transitive and define-ranges-param), root-scoped + $.Values, range index+element, deep object-array sub-fields, and the non-exempting + keywords (maxLength / empty enum / empty pattern) — plus the safe forms (squote, + toJson, toYaml, b64enc, printf %q) and the unknown-function limitation. +module: module +expect: + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuoteForms.aliasScalar" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuoteForms.arrVar" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuoteForms.ctConfig.items" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuoteForms.ctScalar" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuoteForms.deepArr[].spec.name" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuoteForms.defaultStr" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuoteForms.emptyEnumStr" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuoteForms.emptyPatStr" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuoteForms.idxList" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuoteForms.ieList" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuoteForms.matrix[]" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuoteForms.maxLenStr" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuoteForms.moMap[].field" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuoteForms.ptList" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuoteForms.rootScoped" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuoteForms.trimStr" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuoteForms.withVar.field" + count: 1 +expectAbsent: + - linter: templates + rule: openapi-values-quote + textContains: "e2eQuoteForms.squoteStr" + - linter: templates + rule: openapi-values-quote + textContains: "e2eQuoteForms.toJsonStr" + - linter: templates + rule: openapi-values-quote + textContains: "e2eQuoteForms.toYamlStr" + - linter: templates + rule: openapi-values-quote + textContains: "e2eQuoteForms.b64Str" + - linter: templates + rule: openapi-values-quote + textContains: "e2eQuoteForms.pqStr" + - linter: templates + rule: openapi-values-quote + textContains: "e2eQuoteForms.unknownFnStr" diff --git a/test/e2e/testdata/templates/openapi-values-quote-forms/module/module.yaml b/test/e2e/testdata/templates/openapi-values-quote-forms/module/module.yaml new file mode 100644 index 00000000..44f0c6fe --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-forms/module/module.yaml @@ -0,0 +1,2 @@ +name: e2e-quote-forms +namespace: e2e-quote-forms diff --git a/test/e2e/testdata/templates/openapi-values-quote-forms/module/openapi/config-values.yaml b/test/e2e/testdata/templates/openapi-values-quote-forms/module/openapi/config-values.yaml new file mode 100644 index 00000000..cc2ac6c2 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-forms/module/openapi/config-values.yaml @@ -0,0 +1,100 @@ +type: object +properties: + # --- FLAG: template forms --- + ptList: # passthrough over a loop variable + type: array + default: ["a"] + items: { type: string } + idxList: # index into an array + type: array + default: ["a"] + items: { type: string } + withVar: # with over an object variable + type: object + default: { field: x } + properties: + field: { type: string } + arrVar: # array assigned to a variable then ranged + type: array + default: ["a"] + items: { type: string } + aliasScalar: # value copied into a variable then emitted + type: string + default: x + matrix: # array of string arrays (nested range) + type: array + default: [["a"]] + items: + type: array + items: { type: string } + moMap: # map of objects (range key/value, emit subfield) + type: object + default: { k: { field: x } } + additionalProperties: + type: object + properties: + field: { type: string } + ctScalar: # cross-template transitive chain + type: string + default: x + ctConfig: # cross-template: define ranges a param subfield + type: object + default: { items: ["a"] } + properties: + items: + type: array + items: { type: string } + rootScoped: # $.Values.<...> used inside a range + type: string + default: x + ieList: # range $i, $e := ... (index+element form) + type: array + default: ["a"] + items: { type: string } + deepArr: # deeply nested subfield of an object array + type: array + default: [{ spec: { name: x } }] + items: + type: object + properties: + spec: + type: object + properties: + name: { type: string } + maxLenStr: # maxLength does NOT exempt + type: string + default: x + maxLength: 10 + emptyEnumStr: # empty enum does NOT exempt + type: string + default: x + enum: [] + emptyPatStr: # empty pattern does NOT exempt + type: string + default: x + pattern: '' + defaultStr: # | default does not make it safe + type: string + default: x + trimStr: # | trim does not make it safe + type: string + default: x + # --- ABSENT: safe forms --- + squoteStr: + type: string + default: x + toJsonStr: + type: string + default: x + toYamlStr: + type: string + default: x + b64Str: + type: string + default: x + pqStr: # printf %q is safe + type: string + default: x + unknownFnStr: # unknown (non-curated) function -> not reported (limitation) + type: string + default: x diff --git a/test/e2e/testdata/templates/openapi-values-quote-forms/module/openapi/values.yaml b/test/e2e/testdata/templates/openapi-values-quote-forms/module/openapi/values.yaml new file mode 100644 index 00000000..47180da5 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-forms/module/openapi/values.yaml @@ -0,0 +1,4 @@ +x-extend: + schema: config-values.yaml +type: object +properties: {} diff --git a/test/e2e/testdata/templates/openapi-values-quote-forms/module/templates/_helpers.tpl b/test/e2e/testdata/templates/openapi-values-quote-forms/module/templates/_helpers.tpl new file mode 100644 index 00000000..7047a329 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-forms/module/templates/_helpers.tpl @@ -0,0 +1,14 @@ +{{- define "forms.rawScalar" -}} +{{ . }} +{{- end -}} +{{- define "forms.chainA" -}} +{{ include "forms.chainB" . }} +{{- end -}} +{{- define "forms.chainB" -}} +{{ . }} +{{- end -}} +{{- define "forms.listItems" -}} +{{- range .items }} +- {{ . }} +{{- end }} +{{- end -}} diff --git a/test/e2e/testdata/templates/openapi-values-quote-forms/module/templates/flags.yaml b/test/e2e/testdata/templates/openapi-values-quote-forms/module/templates/flags.yaml new file mode 100644 index 00000000..687f0c25 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-forms/module/templates/flags.yaml @@ -0,0 +1,50 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: forms-flags + namespace: e2e-quote-forms +alias: {{ $y := .Values.e2eQuoteForms.aliasScalar }}{{ $y }} +ml: {{ .Values.e2eQuoteForms.maxLenStr }} +ee: {{ .Values.e2eQuoteForms.emptyEnumStr }} +ep: {{ .Values.e2eQuoteForms.emptyPatStr }} +df: {{ .Values.e2eQuoteForms.defaultStr | default "z" }} +tr: {{ .Values.e2eQuoteForms.trimStr | trim }} +idx: {{ index .Values.e2eQuoteForms.idxList 0 }} +ct: {{ include "forms.chainA" .Values.e2eQuoteForms.ctScalar }} +ctc: +{{- include "forms.listItems" .Values.e2eQuoteForms.ctConfig | nindent 2 }} +pt: +{{- range $v := .Values.e2eQuoteForms.ptList }} + - {{ upper $v }} +{{- end }} +av: +{{- $x := .Values.e2eQuoteForms.arrVar }} +{{- range $x }} + - {{ . }} +{{- end }} +ie: +{{- range $i, $e := .Values.e2eQuoteForms.ieList }} + - {{ $e }} +{{- end }} +matrix: +{{- range $row := .Values.e2eQuoteForms.matrix }} +{{- range $c := $row }} + - {{ $c }} +{{- end }} +{{- end }} +deep: +{{- range $d := .Values.e2eQuoteForms.deepArr }} + - {{ $d.spec.name }} +{{- end }} +mo: +{{- range $k, $vv := .Values.e2eQuoteForms.moMap }} + {{ $k }}: {{ $vv.field }} +{{- end }} +{{- $s := .Values.e2eQuoteForms.withVar }} +{{- with $s }} +wv: {{ .field }} +{{- end }} +rs: +{{- range .Values.e2eQuoteForms.ieList }} + - {{ $.Values.e2eQuoteForms.rootScoped }} +{{- end }} diff --git a/test/e2e/testdata/templates/openapi-values-quote-forms/module/templates/safe.yaml b/test/e2e/testdata/templates/openapi-values-quote-forms/module/templates/safe.yaml new file mode 100644 index 00000000..42657203 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-forms/module/templates/safe.yaml @@ -0,0 +1,12 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: forms-safe + namespace: e2e-quote-forms +data: + sq: {{ .Values.e2eQuoteForms.squoteStr | squote }} + tj: {{ .Values.e2eQuoteForms.toJsonStr | toJson }} + ty: {{ .Values.e2eQuoteForms.toYamlStr | toYaml }} + b64: {{ .Values.e2eQuoteForms.b64Str | b64enc }} + pq: {{ printf "%q" .Values.e2eQuoteForms.pqStr }} + uk: {{ indent 2 .Values.e2eQuoteForms.unknownFnStr }} diff --git a/test/e2e/testdata/templates/openapi-values-quote-refs/expected.yaml b/test/e2e/testdata/templates/openapi-values-quote-refs/expected.yaml new file mode 100644 index 00000000..b3d8edfa --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-refs/expected.yaml @@ -0,0 +1,30 @@ +description: > + Schema-alias coverage for openapi-values-quote through the real module loader: + strings reached via $ref / allOf / oneOf without a validation keyword are flagged, + while a $ref that carries a pattern is not. +module: module +expect: + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuoteRefs.refName" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuoteRefs.allOfName" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuoteRefs.oneOfName" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuoteRefs.anyOfName" + count: 1 +expectAbsent: + - linter: templates + rule: openapi-values-quote + textContains: "e2eQuoteRefs.refCode" diff --git a/test/e2e/testdata/templates/openapi-values-quote-refs/module/module.yaml b/test/e2e/testdata/templates/openapi-values-quote-refs/module/module.yaml new file mode 100644 index 00000000..42e03257 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-refs/module/module.yaml @@ -0,0 +1,2 @@ +name: e2e-quote-refs +namespace: e2e-quote-refs diff --git a/test/e2e/testdata/templates/openapi-values-quote-refs/module/openapi/config-values.yaml b/test/e2e/testdata/templates/openapi-values-quote-refs/module/openapi/config-values.yaml new file mode 100644 index 00000000..c2f02e0d --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-refs/module/openapi/config-values.yaml @@ -0,0 +1,28 @@ +type: object +properties: + # FLAGGED: string reached via $ref, no validation + refName: + $ref: '#/definitions/nameType' + # FLAGGED: string via allOf + allOfName: + allOf: + - type: string + # FLAGGED: string branch of oneOf + oneOfName: + oneOf: + - type: string + - type: integer + # FLAGGED: string branch of anyOf + anyOfName: + anyOf: + - type: string + - type: integer + # SAFE: string via $ref that carries a pattern + refCode: + $ref: '#/definitions/codeType' +definitions: + nameType: + type: string + codeType: + type: string + pattern: '^[a-z]+$' diff --git a/test/e2e/testdata/templates/openapi-values-quote-refs/module/openapi/values.yaml b/test/e2e/testdata/templates/openapi-values-quote-refs/module/openapi/values.yaml new file mode 100644 index 00000000..47180da5 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-refs/module/openapi/values.yaml @@ -0,0 +1,4 @@ +x-extend: + schema: config-values.yaml +type: object +properties: {} diff --git a/test/e2e/testdata/templates/openapi-values-quote-refs/module/templates/configmap.yaml b/test/e2e/testdata/templates/openapi-values-quote-refs/module/templates/configmap.yaml new file mode 100644 index 00000000..9021513b --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-refs/module/templates/configmap.yaml @@ -0,0 +1,11 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-quote-refs + namespace: e2e-quote-refs +data: + refName: {{ .Values.e2eQuoteRefs.refName }} + allOfName: {{ .Values.e2eQuoteRefs.allOfName }} + oneOfName: {{ .Values.e2eQuoteRefs.oneOfName }} + anyOfName: {{ .Values.e2eQuoteRefs.anyOfName }} + refCode: {{ .Values.e2eQuoteRefs.refCode }} diff --git a/test/e2e/testdata/templates/openapi-values-quote/expected.yaml b/test/e2e/testdata/templates/openapi-values-quote/expected.yaml index de3c817a..7804264c 100644 --- a/test/e2e/testdata/templates/openapi-values-quote/expected.yaml +++ b/test/e2e/testdata/templates/openapi-values-quote/expected.yaml @@ -1,15 +1,97 @@ description: > - A module template that renders a pattern-less OpenAPI string value unquoted must be - flagged by the openapi-values-quote rule; a string value constrained by a pattern - must not be. + Broad coverage for the openapi-values-quote rule. Strings with no + pattern/enum/format rendered unquoted are flagged (scalar, nullable, nested, + array element); values constrained by pattern/enum/format, integers, quoted and + wrapped usages, values embedded in a larger scalar, and block scalars are not. module: module expect: - linter: templates rule: openapi-values-quote level: error - textContains: "greeting" + textContains: "e2eQuote.greeting" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuote.nickname" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuote.db.host" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuote.extraArgs" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuote.title" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuote.servers[].host" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuote.servers[].aliases" + count: 1 + # embedded in an unquoted scalar -> wrap advice + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuote.endpoint" + count: 1 + # risky value passed to printf + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuote.label" + count: 1 + # unquoted value of a string map (additionalProperties) + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuote.annotations" + count: 1 + # unquoted subfield reached through `with` over a single object + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuote.conn.dsn" + count: 1 + # cross-template: value reaches a module define that renders it unquoted + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eQuote.image" count: 1 expectAbsent: - linter: templates rule: openapi-values-quote - textContains: "code" + textContains: "e2eQuote.code" + - linter: templates + rule: openapi-values-quote + textContains: "e2eQuote.mode" + - linter: templates + rule: openapi-values-quote + textContains: "e2eQuote.createdAt" + - linter: templates + rule: openapi-values-quote + textContains: "e2eQuote.count" + - linter: templates + rule: openapi-values-quote + textContains: "e2eQuote.db.port" + - linter: templates + rule: openapi-values-quote + textContains: "e2eQuote.labels" + - linter: templates + rule: openapi-values-quote + textContains: "e2eQuote.tags" + - linter: templates + rule: openapi-values-quote + textContains: "e2eQuote.servers[].zone" diff --git a/test/e2e/testdata/templates/openapi-values-quote/module/openapi/config-values.yaml b/test/e2e/testdata/templates/openapi-values-quote/module/openapi/config-values.yaml index e5b0ab30..991765a2 100644 --- a/test/e2e/testdata/templates/openapi-values-quote/module/openapi/config-values.yaml +++ b/test/e2e/testdata/templates/openapi-values-quote/module/openapi/config-values.yaml @@ -1,9 +1,110 @@ type: object properties: + # FLAGGED: string with no pattern/enum/format greeting: type: string default: hello + # FLAGGED (embedded in an unquoted scalar): unconstrained string + endpoint: + type: string + default: example.com + # FLAGGED (passed to printf): unconstrained string + label: + type: string + default: app + # FLAGGED (map value via range): map of unconstrained strings + annotations: + type: object + default: + key: value + additionalProperties: + type: string + # FLAGGED (subfield via `with`): single object with an unconstrained string field + conn: + type: object + default: + dsn: postgres + properties: + dsn: + type: string + # SAFE: constrained by a pattern code: type: string pattern: '^[a-z]+$' default: abc + # SAFE: constrained by an enum + mode: + type: string + enum: ["Enabled", "Disabled"] + default: Enabled + # SAFE: constrained by a format + createdAt: + type: string + format: date-time + default: "2020-01-01T00:00:00Z" + # FLAGGED: nullable string, still an unconstrained string + nickname: + type: ["string", "null"] + default: nick + # NOT A STRING: never flagged + count: + type: integer + default: 3 + # nested object with a flagged string leaf and a safe integer leaf + db: + type: object + default: {} + properties: + host: + type: string + default: localhost + port: + type: integer + default: 5432 + # FLAGGED: array whose items are unconstrained strings + extraArgs: + type: array + default: ["--foo"] + items: + type: string + # SAFE: array whose items are constrained by a pattern + labels: + type: array + default: ["ok"] + items: + type: string + pattern: '^[a-z]+$' + # FLAGGED via `with`: unconstrained string scoped by a with block + title: + type: string + default: hi + # SAFE (working quote+range): unconstrained string array, every element quoted + tags: + type: array + default: ["a"] + items: + type: string + # array of objects: string subfields are checked through `range $s := … }}{{ $s.field }}`, + # and a string-array subfield is checked through a nested range (range-in-range) + servers: + type: array + default: + - host: h + zone: a + aliases: ["x"] + items: + type: object + properties: + host: + type: string + zone: + type: string + enum: ["a", "b"] + aliases: + type: array + items: + type: string + # FLAGGED (cross-template): passed to a module define that renders it unquoted + image: + type: string + default: nginx diff --git a/test/e2e/testdata/templates/openapi-values-quote/module/templates/_helpers.tpl b/test/e2e/testdata/templates/openapi-values-quote/module/templates/_helpers.tpl new file mode 100644 index 00000000..34138406 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote/module/templates/_helpers.tpl @@ -0,0 +1,3 @@ +{{- define "e2e-quote.raw" -}} +{{ . }} +{{- end -}} diff --git a/test/e2e/testdata/templates/openapi-values-quote/module/templates/args.yaml b/test/e2e/testdata/templates/openapi-values-quote/module/templates/args.yaml new file mode 100644 index 00000000..f4698ac3 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote/module/templates/args.yaml @@ -0,0 +1,20 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-quote-args + namespace: e2e-quote +# FLAGGED: element of an unconstrained string array, bound to a range variable +extraArgs: +{{- range $arg := .Values.e2eQuote.extraArgs }} + - {{ $arg }} +{{- end }} +# SAFE: element of a pattern-constrained string array (dot-binding form) +safeArgs: +{{- range .Values.e2eQuote.labels }} + - {{ . }} +{{- end }} +# SAFE: unconstrained string array, elements quoted via the range variable +tags: +{{- range $t := .Values.e2eQuote.tags }} + - {{ $t | quote }} +{{- end }} diff --git a/test/e2e/testdata/templates/openapi-values-quote/module/templates/configmap.yaml b/test/e2e/testdata/templates/openapi-values-quote/module/templates/configmap.yaml index 9bad874a..2448a111 100644 --- a/test/e2e/testdata/templates/openapi-values-quote/module/templates/configmap.yaml +++ b/test/e2e/testdata/templates/openapi-values-quote/module/templates/configmap.yaml @@ -4,7 +4,32 @@ metadata: name: e2e-quote namespace: e2e-quote data: - # pattern-less string rendered unquoted -> must be flagged + # FLAGGED: pattern-less string rendered unquoted greeting: {{ .Values.e2eQuote.greeting }} - # string with a validation pattern -> must NOT be flagged + # SAFE: same string, piped through quote + greetingQuoted: {{ .Values.e2eQuote.greeting | quote }} + # SAFE: same string, wrapped in quotes + greetingWrapped: "{{ .Values.e2eQuote.greeting }}" + # SAFE: embedded in a quoted scalar + greetingURL: "https://{{ .Values.e2eQuote.greeting }}/health" + # FLAGGED: embedded in an UNQUOTED scalar -> wrap the whole value + endpointURL: https://{{ .Values.e2eQuote.endpoint }}/health + # FLAGGED: risky value passed to printf + banner: {{ printf "hello-%s" .Values.e2eQuote.label }} + # SAFE: constrained by a pattern code: {{ .Values.e2eQuote.code }} + # SAFE: constrained by an enum + mode: {{ .Values.e2eQuote.mode }} + # SAFE: constrained by a format + createdAt: {{ .Values.e2eQuote.createdAt }} + # FLAGGED: nullable string rendered unquoted + nickname: {{ .Values.e2eQuote.nickname }} + # SAFE: not a string + count: {{ .Values.e2eQuote.count }} + # FLAGGED: nested string rendered unquoted + host: {{ .Values.e2eQuote.db.host }} + # SAFE: nested integer + port: {{ .Values.e2eQuote.db.port }} + # SAFE: inside a YAML block scalar (already a literal string) + script: | + echo {{ .Values.e2eQuote.greeting }} diff --git a/test/e2e/testdata/templates/openapi-values-quote/module/templates/include.yaml b/test/e2e/testdata/templates/openapi-values-quote/module/templates/include.yaml new file mode 100644 index 00000000..31adc45b --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote/module/templates/include.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-quote-include + namespace: e2e-quote +data: + # FLAGGED: risky value rendered unquoted by the module template "e2e-quote.raw" + image: {{ include "e2e-quote.raw" .Values.e2eQuote.image }} diff --git a/test/e2e/testdata/templates/openapi-values-quote/module/templates/maps-and-with.yaml b/test/e2e/testdata/templates/openapi-values-quote/module/templates/maps-and-with.yaml new file mode 100644 index 00000000..b9f71f3f --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote/module/templates/maps-and-with.yaml @@ -0,0 +1,15 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-quote-maps + namespace: e2e-quote +annotations: +# FLAGGED: unquoted value of a string map (additionalProperties), via range +{{- range $k, $v := .Values.e2eQuote.annotations }} + {{ $k }}: {{ $v }} +{{- end }} +data: +# FLAGGED: unquoted string subfield reached through `with` over a single object +{{- with .Values.e2eQuote.conn }} + dsn: {{ .dsn }} +{{- end }} diff --git a/test/e2e/testdata/templates/openapi-values-quote/module/templates/servers.yaml b/test/e2e/testdata/templates/openapi-values-quote/module/templates/servers.yaml new file mode 100644 index 00000000..cf4b2d03 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote/module/templates/servers.yaml @@ -0,0 +1,17 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-quote-servers + namespace: e2e-quote +hosts: +{{- range $s := .Values.e2eQuote.servers }} + # FLAGGED: unquoted string subfield of an object array element + - host: {{ $s.host }} + # SAFE: subfield constrained by an enum + zone: {{ $s.zone }} + aliases: + # FLAGGED (range-in-range): unquoted element of a string-array subfield + {{- range $a := $s.aliases }} + - {{ $a }} + {{- end }} +{{- end }} diff --git a/test/e2e/testdata/templates/openapi-values-quote/module/templates/with.yaml b/test/e2e/testdata/templates/openapi-values-quote/module/templates/with.yaml new file mode 100644 index 00000000..d4285f88 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote/module/templates/with.yaml @@ -0,0 +1,10 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-quote-with + namespace: e2e-quote +{{- with .Values.e2eQuote.title }} +data: + # FLAGGED: scalar string scoped by `with`, emitted unquoted + title: {{ . }} +{{- end }} From 2d5610c11f24476c4b47b0dffa614a2d6a05a7f8 Mon Sep 17 00:00:00 2001 From: Pavel Okhlopkov Date: Wed, 19 Aug 2026 22:48:16 +0300 Subject: [PATCH 3/8] fix docs Signed-off-by: Pavel Okhlopkov --- pkg/linters/templates/README.md | 25 +++++++++++++------ .../templates/rules/openapi_values_quote.go | 17 ++++++++----- .../rules/openapi_values_quote_test.go | 18 +++++++++++++ 3 files changed, 47 insertions(+), 13 deletions(-) diff --git a/pkg/linters/templates/README.md b/pkg/linters/templates/README.md index fcd74932..d1379563 100644 --- a/pkg/linters/templates/README.md +++ b/pkg/linters/templates/README.md @@ -24,7 +24,7 @@ Proper template validation prevents runtime issues, ensures applications are pro | [enabled-modules](#enabled-modules) | Detects usage of `.Values.global.enabledModules` in templates | ✅ | enabled | | [webhook-configuration-annotations](#webhook-configuration-annotations) | Checks webhook configurations have werf.io/weight or deploy-dependency annotations | ✅ | enabled | | [mount-points](#mount-points) | Validates that mount-points.yaml directories are used as volumeMounts in pod controllers | ✅ | enabled | -| [openapi-values-quote](#openapi-values-quote) | Requires templates to quote OpenAPI string values that have no validation pattern | ✅ | enabled | +| [openapi-values-quote](#openapi-values-quote) | Requires templates to quote OpenAPI string values that have no `pattern`/`enum`/`format` | ✅ | enabled | "Configurable" means that this rule can be configured using the `.dmtlint.yaml` file, including customizing the rule's parameters and/or disabling the rule. @@ -2039,7 +2039,7 @@ linters-settings: ### openapi-values-quote -**Purpose:** Ensures that module OpenAPI string values without a validation pattern are always quoted when rendered into templates. An unconstrained string can contain characters that break YAML or silently change the parsed type (for example `123`, `true`, `on`, a value with a leading `0`, or one containing `:` or `#`), so it must be quoted at the point of use. +**Purpose:** Ensures that module OpenAPI string values with no validation keyword (`pattern`, `enum`, or `format`) are always quoted when rendered into templates. An unconstrained string can contain characters that break YAML or silently change the parsed type (for example `123`, `true`, `on`, a value with a leading `0`, or one containing `:` or `#`), so it must be quoted at the point of use. **Description:** @@ -2055,7 +2055,7 @@ Both the direct form `.Values..` and the root-scoped `$.Values - items of a string array (`type: array` with `items: {type: string}`), checked through `range` (`{{ range … }}{{ . }}{{ end }}`); - a string sub-field (at any depth) of an array-of-objects element, checked through a named range: `{{ range $s := .Values.mod.servers }}{{ $s.host }}{{ end }}` (and the dot-binding form `{{ range .Values.mod.servers }}{{ .host }}{{ end }}`); - an element of a string-array sub-field, checked through a nested range (range-in-range) at any nesting depth: `{{ range $s := .Values.mod.servers }}{{ range $a := $s.aliases }}{{ $a }}{{ end }}{{ end }}`; -- values of a string map (`type: object` with `additionalProperties: {type: string}`), checked through `{{ range $k, $v := .Values.mod.labels }}{{ $v }}{{ end }}` (map-of-objects and nested maps too); +- values of a string map (`type: object` with `additionalProperties: {type: string}`), checked through `{{ range $k, $v := .Values.mod.labels }}{{ $v }}{{ end }}` (a map of objects works too, via `{{ $v.field }}`); - elements of an array of string arrays (`items: {type: array, items: {type: string}}`), checked through a nested range; - a string scoped by `with`, both over a scalar (`{{ with .Values.mod.foo }}{{ . }}{{ end }}`) and over a single object (`{{ with .Values.mod.db }}{{ .host }}{{ end }}`); - a value copied into a template variable and then emitted (`{{ $x := .Values.mod.foo }}… {{ $x }}`), including aliases of loop element variables and an array bound to a variable and later ranged (`{{ $x := .Values.mod.list }}{{ range $x }}{{ . }}{{ end }}`); @@ -2069,7 +2069,16 @@ Both the direct form `.Values..` and the root-scoped `$.Values The rule reports both a **standalone** value (`key: {{ … }}` / `- {{ … }}`, fix: add `| quote`) and a value **embedded** in a larger unquoted scalar (`host: prefix-{{ … }}`, fix: wrap the whole value in quotes) — the message tells you which fix applies. A risky value passed to a common passthrough function — a string transform (`{{ printf "%s" .Values.mod.foo }}`, `{{ upper $v }}`) or an array/map element accessor (`{{ index .Values.mod.list 0 }}`) — is reported as well, for both `.Values` references and variables. A value used only in a condition (`{{ if … }}`) is not a rendered value and is not reported. -**Known limitations (not reported):** a value inside a YAML block scalar (`|` / `>`), where it is already part of a string and must **not** be quoted; a value passed to an **external** template (helm_lib and other charts, whose bodies are not scanned); a map **key** emitted as `{{ $k }}:` (indistinguishable from a numeric array index without extra type info); a value reached through dynamic access (`dig`/`get`/`pluck`, or a `dict` built inline and passed on); and a `{{ … }}` action that spans multiple physical lines. +**Known limitations (not reported):** + +- a value inside a YAML block scalar (`|` / `>`), where it is already part of a string and must **not** be quoted; +- a value passed to a function that is **not** in the recognised transform/accessor set, nested inside another call (`{{ printf "%s" (index .Values.mod.list 0) }}`), or used as the **subject** of `with`/`range` (`{{ with (index .Values.mod.servers 0) }}`); +- a value a module `define` renders **through a function** (e.g. `{{ printf "%s" . }}` inside the define — only direct `{{ . }}` / `{{ .field }}` / `{{ range .field }}` forms are traced); +- a value passed to an **external** template (helm_lib and other charts, whose bodies are not scanned); +- a map **key** emitted as `{{ $k }}:` (indistinguishable from a numeric array index without extra type info); +- the values of a **nested map** (`additionalProperties` whose value is itself a map); +- a value reached through **dynamic access** (`dig` / `get` / `pluck`, or a `dict` built inline and passed on); +- a `{{ … }}` action that spans multiple physical lines. **Why it matters:** @@ -2128,7 +2137,7 @@ hosts: **Configuration:** -Set the impact level or exclude specific value paths (matched against the dotted path relative to the module values root, e.g. `foo.bar`): +Set the impact level or exclude specific value paths. An exclude is matched (exact string) against the value path **exactly as it appears in the finding message** — a dotted path relative to the module values root, with `[]` marking array elements and array-of-object sub-fields: ```yaml # .dmtlint.yaml @@ -2139,8 +2148,10 @@ linters-settings: impact: error exclude-rules: openapi-values-quote: - - internal.someLegacyField - - extraArgs + - internal.someLegacyField # a scalar path + - extraArgs # a string array (excludes its elements) + - servers[].host # one sub-field of an array of objects + - servers # the whole array (excludes every sub-field) ``` diff --git a/pkg/linters/templates/rules/openapi_values_quote.go b/pkg/linters/templates/rules/openapi_values_quote.go index 801c13da..eb67505d 100644 --- a/pkg/linters/templates/rules/openapi_values_quote.go +++ b/pkg/linters/templates/rules/openapi_values_quote.go @@ -442,23 +442,28 @@ func (r *OpenAPIValuesQuoteRule) checkScopedEmissions( } // reportScoped emits the finding for a scoped emission, worded for a scalar value, an -// array element, or a string sub-field of an array-of-objects element. +// array element, or a string sub-field of an array-of-objects element. The full value +// path (e.g. `servers[].host`) is checked against the exclude list so a specific +// sub-field can be excluded, not only its enclosing array. func (r *OpenAPIValuesQuoteRule) reportScoped( relPath string, line int, valuesKey string, scope *loopScope, sub string, wrap bool, errorList *errors.LintRuleErrorsList, ) { + path := emissionPath(scope, sub) + if !r.Enabled(path) { + return + } + tail := "is an OpenAPI string without a validation pattern (pattern/enum/format) " + quoteAdvice(wrap) el := errorList.WithFilePath(relPath).WithLineNumber(line) switch { case scope.kind == "array": - el.Errorf("array element from '.Values.%s.%s' %s", valuesKey, scope.valPath, tail) + el.Errorf("array element from '.Values.%s.%s' %s", valuesKey, path, tail) case scope.kind == "object" && scope.elemArray: - el.Errorf("array element field '.Values.%s.%s[].%s' %s", valuesKey, scope.valPath, sub, tail) - case scope.kind == "object": - el.Errorf("value '.Values.%s.%s.%s' %s", valuesKey, scope.valPath, sub, tail) + el.Errorf("array element field '.Values.%s.%s' %s", valuesKey, path, tail) default: - el.Errorf("value '.Values.%s.%s' %s", valuesKey, scope.valPath, tail) + el.Errorf("value '.Values.%s.%s' %s", valuesKey, path, tail) } } diff --git a/pkg/linters/templates/rules/openapi_values_quote_test.go b/pkg/linters/templates/rules/openapi_values_quote_test.go index aa358c33..ed24e349 100644 --- a/pkg/linters/templates/rules/openapi_values_quote_test.go +++ b/pkg/linters/templates/rules/openapi_values_quote_test.go @@ -1024,6 +1024,24 @@ properties: }, wantCount: 0, }, + { + name: "object array subfield can be excluded by its full path", + valuesSchema: `type: object +properties: + servers: + type: array + items: + type: object + properties: + host: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "servers:\n{{- range $s := .Values." + valuesKey + ".servers }}\n host: {{ $s.host }}\n{{- end }}\n", + }, + excludes: []string{"servers[].host"}, + wantCount: 0, + }, { name: "unquoted subfield of an object array nested in an object array is flagged", valuesSchema: `type: object From 9a020bba1cdccd565e1e81df592f8b0e597ba1ab Mon Sep 17 00:00:00 2001 From: Pavel Okhlopkov Date: Thu, 20 Aug 2026 14:25:19 +0300 Subject: [PATCH 4/8] add stats Signed-off-by: Pavel Okhlopkov --- cmd/dmt/main.go | 1 + internal/manager/manager.go | 8 +- internal/manager/statistics.go | 254 ++++++++++++++++++++++++++++ internal/manager/statistics_test.go | 128 ++++++++++++++ 4 files changed, 390 insertions(+), 1 deletion(-) create mode 100644 internal/manager/statistics.go create mode 100644 internal/manager/statistics_test.go diff --git a/cmd/dmt/main.go b/cmd/dmt/main.go index 4737512f..5b221247 100644 --- a/cmd/dmt/main.go +++ b/cmd/dmt/main.go @@ -91,6 +91,7 @@ func runLint(dir string) error { } mng.PrintResult() + mng.PrintStatistics() metrics.SetDmtInfo() metrics.SetLinterWarningsMetrics(cfg.GlobalSettings) diff --git a/internal/manager/manager.go b/internal/manager/manager.go index 936d06b6..47bef647 100644 --- a/internal/manager/manager.go +++ b/internal/manager/manager.go @@ -26,6 +26,7 @@ import ( "strings" "sync" "text/tabwriter" + "time" "github.com/fatih/color" "github.com/kyokomi/emoji" @@ -76,6 +77,10 @@ type Manager struct { Modules []*modules.Module errors *errors.LintRuleErrorsList + + // startedAt marks the beginning of the run; PrintStatistics reports the + // wall-clock time elapsed since it, matching the mirror summary's Elapsed line. + startedAt time.Time } func NewManager(dir string, rootConfig *config.RootConfig) *Manager { @@ -83,7 +88,8 @@ func NewManager(dir string, rootConfig *config.RootConfig) *Manager { m := &Manager{ cfg: rootConfig, - errors: errors.NewLintRuleErrorsList().WithMaxLevel(&managerLevel), + errors: errors.NewLintRuleErrorsList().WithMaxLevel(&managerLevel), + startedAt: time.Now(), } return m.initManager(dir) diff --git a/internal/manager/statistics.go b/internal/manager/statistics.go new file mode 100644 index 00000000..c510bdd7 --- /dev/null +++ b/internal/manager/statistics.go @@ -0,0 +1,254 @@ +/* +Copyright 2025 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package manager + +import ( + "cmp" + "fmt" + "slices" + "strings" + "time" + "unicode/utf8" + + "github.com/fatih/color" + + "github.com/deckhouse/dmt/pkg" +) + +// The statistics summary is rendered as a single framed block that intentionally +// mirrors the look of the deckhouse-cli `mirror` pull/push summaries: the same +// framed box, the same cyan labels padded to a fixed column, the same semantic +// accent colours, and a trailing state + Elapsed line. Keeping the primitives in +// step with that summary is what makes the two tools feel like one family. +const ( + // frameWidth is the inner width of the summary box, in runes. + frameWidth = 56 + // labelWidth aligns the category labels in the summary body. + labelWidth = 11 + // nameWidth left-aligns per-linter names. + nameWidth = 30 +) + +// Semantic accent colours for the summary. fatih/color disables them when stdout +// is not a TTY or NO_COLOR is set, so escape codes never reach pipes or files. +// +// Apply every colour AFTER width padding (padLabel, %-30s): the codes are +// zero-width on screen but count toward fmt's field widths and break columns. +var ( + cFrame = color.New(color.FgHiBlack).SprintFunc() // box borders - recede + cTitle = color.New(color.FgCyan, color.Bold).SprintFunc() // block title + cLabel = color.New(color.FgCyan).SprintFunc() // category labels (scan anchors) + cCount = color.New(color.Bold).SprintFunc() // primary numbers + cDim = color.New(color.FgHiBlack).SprintFunc() // units and secondary text + cGood = color.New(color.FgGreen).SprintFunc() // clean run + cWarn = color.New(color.FgYellow).SprintFunc() // attention (warnings) + cBad = color.New(color.FgRed).SprintFunc() // failure (errors) +) + +// bar returns the coloured left border of a body line. +func bar() string { return cFrame("║") } + +// writeTopBorder writes the framed-block top border with the given title. +func writeTopBorder(b *strings.Builder, title string) { + prefix := "╔══ " + suffix := " " + used := utf8.RuneCountInString(prefix) + utf8.RuneCountInString(title) + utf8.RuneCountInString(suffix) + + pad := max(0, frameWidth-used) + + fmt.Fprintf(b, "%s%s%s%s\n", cFrame(prefix), cTitle(title), suffix, cFrame(strings.Repeat("═", pad))) +} + +// padLabel formats a category label as a fixed-width "Name:" column. +func padLabel(name string) string { + return fmt.Sprintf("%-*s", labelWidth, name+":") +} + +// formatDuration renders an elapsed duration compactly, keeping millisecond +// precision for sub-second runs (so a fast run does not report "0s"). +func formatDuration(d time.Duration) string { + if d < time.Second { + return d.Round(time.Millisecond).String() + } + + return d.Round(time.Second).String() +} + +// linterStat is one linter's contribution to the findings. +type linterStat struct { + name string + count int +} + +// statistics is the end-of-lint accounting handed to the renderer. +type statistics struct { + // modules is the number of modules that were linted. + modules int + // critical, errors, warnings and ignored are the severity breakdown of all + // collected findings. + critical int + errors int + warnings int + ignored int + // total is the sum of all findings across every severity. + total int + // byLinter holds the per-linter breakdown, most findings first. + byLinter []linterStat + // elapsed is the wall-clock duration of the run. + elapsed time.Duration +} + +// collectStatistics tallies every collected finding by severity and by linter. +// Counts are taken over all findings regardless of the --hide-warnings / +// --show-ignored display flags: the summary is meant to give the full picture. +func (m *Manager) collectStatistics() statistics { + s := statistics{ + modules: len(m.Modules), + elapsed: time.Since(m.startedAt), + } + + perLinter := make(map[string]int) + + errs := m.errors.GetErrors() + for idx := range errs { + s.total++ + + switch errs[idx].Level { + case pkg.Critical: + s.critical++ + case pkg.Error: + s.errors++ + case pkg.Warn: + s.warnings++ + case pkg.Ignored: + s.ignored++ + } + + perLinter[errs[idx].LinterID]++ + } + + for name, count := range perLinter { + s.byLinter = append(s.byLinter, linterStat{name: name, count: count}) + } + + slices.SortFunc(s.byLinter, func(a, b linterStat) int { + return cmp.Or( + cmp.Compare(b.count, a.count), + cmp.Compare(a.name, b.name), + ) + }) + + return s +} + +// PrintStatistics prints the end-of-lint statistics as a framed summary block +// styled identically to the deckhouse-cli mirror summaries. It is meant to be +// called after PrintResult, once all findings have been listed. +func (m *Manager) PrintStatistics() { + fmt.Println(renderStatistics(m.collectStatistics())) +} + +// renderStatistics formats the statistics as a single multi-line, framed block. +// +// Example output (colour stripped): +// +// ╔══ Lint summary ═══════════════════════════════════════ +// ║ Modules: 3 +// ║ +// ║ Critical: 0 +// ║ Errors: 5 +// ║ Warnings: 12 +// ║ Ignored: 2 +// ║ +// ║ By linter: +// ║ templates 12 +// ║ openapi 3 +// ║ no-cyrillic 2 +// ║ +// ║ Total: 19 findings +// ║ +// ║ Lint failed; 5 problem(s) must be fixed. +// ║ Elapsed: 1.2s +// ╚═══════════════════════════════════════════════════════ +// +// The title switches to "Lint failed" and the state line turns red when there +// are error/critical findings; a clean run reports "No problems found." in green. +func renderStatistics(s statistics) string { + var b strings.Builder + + b.WriteByte('\n') + + failed := s.critical+s.errors > 0 + + title := "Lint summary" + if failed { + title = "Lint failed" + } + + writeTopBorder(&b, title) + + // Modules scanned. + fmt.Fprintf(&b, "%s %s %s\n", bar(), cLabel(padLabel("Modules")), cCount(fmt.Sprint(s.modules))) + fmt.Fprintln(&b, bar()) + + // Severity breakdown. A zero count is dimmed so a clean run reads calmly and + // the eye is drawn only to the severities that actually fired. + writeSeverity(&b, "Critical", s.critical, cBad) + writeSeverity(&b, "Errors", s.errors, cBad) + writeSeverity(&b, "Warnings", s.warnings, cWarn) + writeSeverity(&b, "Ignored", s.ignored, cDim) + + // Per-linter breakdown, most findings first. Omitted entirely on a clean run. + if len(s.byLinter) > 0 { + fmt.Fprintln(&b, bar()) + fmt.Fprintf(&b, "%s %s\n", bar(), cLabel("By linter")) + + for _, ls := range s.byLinter { + fmt.Fprintf(&b, "%s %-*s %s\n", bar(), nameWidth, ls.name, cCount(fmt.Sprint(ls.count))) + } + } + + fmt.Fprintln(&b, bar()) + fmt.Fprintf(&b, "%s %s %s\n", bar(), cLabel(padLabel("Total")), cCount(fmt.Sprintf("%d findings", s.total))) + + fmt.Fprintln(&b, bar()) + + switch { + case failed: + fmt.Fprintf(&b, "%s %s\n", bar(), cBad(fmt.Sprintf("Lint failed; %d problem(s) must be fixed.", s.critical+s.errors))) + case s.warnings > 0: + fmt.Fprintf(&b, "%s %s\n", bar(), cWarn(fmt.Sprintf("Completed with %d warning(s).", s.warnings))) + default: + fmt.Fprintf(&b, "%s %s\n", bar(), cGood("No problems found.")) + } + + fmt.Fprintf(&b, "%s %s\n", bar(), cDim("Elapsed: "+formatDuration(s.elapsed))) + b.WriteString(cFrame("╚" + strings.Repeat("═", frameWidth-1))) + + return b.String() +} + +// writeSeverity renders one severity line, e.g. `║ Errors: 5`. The count is +// dimmed when zero and coloured with activeColor otherwise. +func writeSeverity(b *strings.Builder, name string, count int, activeColor func(...any) string) { + value := activeColor(fmt.Sprint(count)) + if count == 0 { + value = cDim(fmt.Sprint(count)) + } + + fmt.Fprintf(b, "%s %s %s\n", bar(), cLabel(padLabel(name)), value) +} diff --git a/internal/manager/statistics_test.go b/internal/manager/statistics_test.go new file mode 100644 index 00000000..17227bc0 --- /dev/null +++ b/internal/manager/statistics_test.go @@ -0,0 +1,128 @@ +/* +Copyright 2025 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package manager + +import ( + "strings" + "testing" + "time" + "unicode/utf8" + + "github.com/fatih/color" + "github.com/stretchr/testify/require" +) + +// withNoColor disables ANSI colour for the duration of the test so the rendered +// summary is asserted on its plain text, then restores the previous setting. +func withNoColor(t *testing.T) { + t.Helper() + + orig := color.NoColor + color.NoColor = true + + t.Cleanup(func() { color.NoColor = orig }) +} + +func TestRenderStatistics_Failed(t *testing.T) { + withNoColor(t) + + out := renderStatistics(statistics{ + modules: 2, + errors: 5, + warnings: 3, + ignored: 1, + total: 9, + byLinter: []linterStat{ + {name: "templates", count: 5}, + {name: "openapi", count: 3}, + {name: "module", count: 1}, + }, + elapsed: 250 * time.Millisecond, + }) + + // Failed state: title and the red state line reflect the error count. + require.Contains(t, out, "Lint failed") + require.Contains(t, out, "Lint failed; 5 problem(s) must be fixed.") + + // Severity breakdown. + require.Contains(t, out, "Modules: 2") + require.Contains(t, out, "Errors: 5") + require.Contains(t, out, "Warnings: 3") + require.Contains(t, out, "Ignored: 1") + + // Per-linter breakdown, most findings first. + require.Contains(t, out, "By linter") + require.Contains(t, out, "templates") + require.Contains(t, out, "Total: 9 findings") + require.Contains(t, out, "Elapsed: 250ms") +} + +func TestRenderStatistics_WarningsOnly(t *testing.T) { + withNoColor(t) + + out := renderStatistics(statistics{ + modules: 1, + warnings: 4, + total: 4, + byLinter: []linterStat{{name: "container", count: 4}}, + elapsed: 500 * time.Millisecond, + }) + + // No errors: it is a summary, not a failure, and the state line is the + // warnings note rather than a failure or a clean-run message. + require.Contains(t, out, "Lint summary") + require.NotContains(t, out, "Lint failed") + require.Contains(t, out, "Completed with 4 warning(s).") + require.NotContains(t, out, "No problems found.") +} + +func TestRenderStatistics_Clean(t *testing.T) { + withNoColor(t) + + out := renderStatistics(statistics{ + modules: 3, + total: 0, + elapsed: 42 * time.Millisecond, + }) + + require.Contains(t, out, "Lint summary") + require.Contains(t, out, "No problems found.") + require.Contains(t, out, "Total: 0 findings") + + // A clean run omits the per-linter breakdown entirely. + require.NotContains(t, out, "By linter") +} + +// TestRenderStatistics_FrameGeometry pins the framed box to a fixed inner width, +// matching the mirror summary: the top and bottom borders are exactly frameWidth +// runes wide, so the two tools' summaries line up. +func TestRenderStatistics_FrameGeometry(t *testing.T) { + withNoColor(t) + + out := renderStatistics(statistics{modules: 1, errors: 1, total: 1, elapsed: time.Second}) + + lines := strings.Split(strings.Trim(out, "\n"), "\n") + require.NotEmpty(t, lines) + + top := lines[0] + bottom := lines[len(lines)-1] + + require.True(t, strings.HasPrefix(top, "╔══ "), "top border starts the frame") + require.True(t, strings.HasPrefix(bottom, "╚"), "bottom border closes the frame") + require.Equal(t, frameWidth, utf8.RuneCountInString(top), "top border width") + require.Equal(t, frameWidth, utf8.RuneCountInString(bottom), "bottom border width") +} From c841e30c3e5f267bb34125f8161610a068bef3e9 Mon Sep 17 00:00:00 2001 From: Pavel Okhlopkov Date: Thu, 20 Aug 2026 15:16:08 +0300 Subject: [PATCH 5/8] add tests Signed-off-by: Pavel Okhlopkov --- .../README.md | 88 +++++++++++++ .../expected.yaml | 74 +++++++++++ .../module/module.yaml | 2 + .../module/openapi/config-values.yaml | 59 +++++++++ .../module/openapi/values.yaml | 4 + .../module/templates/haz-coercion.yaml | 12 ++ .../module/templates/haz-doc-end.yaml | 10 ++ .../module/templates/haz-flow.yaml | 11 ++ .../module/templates/haz-indicators.yaml | 12 ++ .../module/templates/haz-key-injection.yaml | 9 ++ .../module/templates/haz-truncation.yaml | 9 ++ .../module/templates/safe.yaml | 12 ++ .../expected.yaml | 13 ++ .../module/module.yaml | 2 + .../module/openapi/config-values.yaml | 81 ++++++++++++ .../module/openapi/values.yaml | 4 + .../module/templates/configmap.yaml | 26 ++++ .../expected.yaml | 60 +++++++++ .../module/module.yaml | 2 + .../module/openapi/config-values.yaml | 123 ++++++++++++++++++ .../module/openapi/values.yaml | 4 + .../module/templates/safe.yaml | 21 +++ .../module/templates/vuln-list.yaml | 11 ++ .../module/templates/vuln-map.yaml | 10 ++ .../module/templates/vuln-nested.yaml | 8 ++ .../module/templates/vuln-scalar.yaml | 9 ++ .../module/templates/vuln-servers.yaml | 11 ++ .../module/templates/vuln-with.yaml | 11 ++ 28 files changed, 698 insertions(+) create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/README.md create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/expected.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/module.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/openapi/config-values.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/openapi/values.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/haz-coercion.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/haz-doc-end.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/haz-flow.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/haz-indicators.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/haz-key-injection.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/haz-truncation.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/safe.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-injection-safe/expected.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-injection-safe/module/module.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-injection-safe/module/openapi/config-values.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-injection-safe/module/openapi/values.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-injection-safe/module/templates/configmap.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-injection/expected.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/module.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/openapi/config-values.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/openapi/values.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/safe.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/vuln-list.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/vuln-map.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/vuln-nested.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/vuln-scalar.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/vuln-servers.yaml create mode 100644 test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/vuln-with.yaml diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/README.md b/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/README.md new file mode 100644 index 00000000..7ee7991b --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/README.md @@ -0,0 +1,88 @@ +# YAML-injection e2e coverage for `openapi-values-quote` + +Three e2e cases exercise the `openapi-values-quote` rule against the YAML-injection +threat model. The rule's guarantee is simple: an unconstrained OpenAPI string (no +`pattern`/`enum`/`format`) rendered **unquoted** is a hazard and must be flagged; the +same value is safe when quoted (`| quote`, wrapped, or a YAML-safe function) or when a +schema keyword restricts its character set. + +| Case | Focus | +|------|-------| +| `openapi-values-quote-yaml-injection` | document-separator `\n---\n` (rogue-document injection), across scalar / array / map / nested / `with` / array-of-objects vectors | +| `openapi-values-quote-yaml-injection-safe` | negative control — the same dangerous values, all quoted/constrained → the rule must be silent and the render must emit no rogue document | +| `openapi-values-quote-yaml-hazards` | the rest of the family — everything below marked ✅ | + +## Covered + +Every payload below renders to **valid YAML** (so the module loads and the rule finding +can be asserted). The "render effect" column was verified against dmt's real render +(nelm + its YAML parser); the e2e case asserts the rule fires and the module loads. + +| Class | Payload (unquoted) | Verified render effect | +|-------|--------------------|------------------------| +| Document separator | `foo\n---\n…` | injects a whole new document (a `Secret` appears in the object store) | +| Document end | `foo\n...\n` | `...` truncates the document — trailing fields silently dropped | +| Key injection | `foo\n key: val` (indented to the field) | injects a sibling key into the current mapping (`injected: pwned`) — no new document needed | +| Bool coercion | `no` / `on` / `yes` | parsed as a boolean (`no` → `false`) — the "Norway problem" | +| Int coercion | `0755` / `1234567890` | parsed as an integer (`0755` → octal `493`) | +| Float coercion | `1.10` | parsed as a float (`1.1` — trailing zero lost) | +| Null coercion | `~` / `null` / empty | parsed as null | +| Comment truncation | `visible # secret` | everything after ` #` dropped as a comment | +| Flow sequence | `[a, b]` | string becomes a list | +| Flow mapping | `{a: b}` | string becomes a map | +| Tag indicator | `!badtag` | leading `!` read as a YAML tag; value rendered empty | +| Anchor indicator | `&a value` | leading `&` read as an anchor; token stripped | + +## NOT covered — and why + +### 1. Parse-breaking payloads abort module creation (cannot assert the rule) + +Payloads that render to **invalid** YAML make dmt fail `NewModule` with a single +`manager: cannot create module` finding — the per-linter phase never runs, so the +`openapi-values-quote` finding cannot be observed in an e2e case that bakes the payload +into a value default. Verified fatal: + +- bare `key: value` (colon-space) — `mapping value is not allowed in this context` +- leading `@` or `` ` `` — reserved indicator +- `*name` — reference to an undefined alias +- a column-0 `foo\nkey: val` (key injection whose indentation does **not** match) +- (same class: unbalanced flow `[a, b`, a leading tab that breaks indentation) + +This is a limitation of the **synthetic-default e2e setup**, not of the rule: the rule +reads raw template source, so in a real module (real values, not a baked default) it +still flags these unquoted usages before they ever reach a cluster. The danger of these +payloads is instead the render abort itself. + +### 2. The framework asserts on findings, not on rendered shapes + +`expected.yaml` matches findings by `linter` / `rule` / `level` / `textContains`. It +cannot assert "the value became a bool", "a key was injected", or "the document was +truncated" — those render effects are verified by hand (see the probe approach in the +commit history) and documented in the case, but the machine assertion is limited to +"the rule flagged the unquoted usage" and "the module still rendered". + +### 3. Out of the rule's scope (documented rule limitations) + +The rule intentionally does **not** flag these, so they are not coverable as +"rule catches it" (see the rule's own limitations in +`pkg/linters/templates/README.md`): + +- a value inside a block scalar (`|` / `>`) — already a literal string, must not be quoted +- a value passed through Helm `tpl` (template injection) or to an **external** template (helm_lib) +- `toYaml | nindent N` indentation attacks (whole-subtree injection) +- dynamic access (`dig` / `get` / `pluck`, `index` nested in another call) +- a map **key** emitted as `{{ $k }}:` +- a `{{ … }}` action spanning multiple physical lines + +### 4. Parser-dependent coercions that do not fire here + +Measured against dmt's actual parser: the YAML-1.1 boolean set (`yes/no/on/off`) **does** +coerce, but **timestamps** (`2020-01-01`) and **base-60 / sexagesimal** (`1:2:3`) do +**not** — they stay strings. So there is deliberately no coercion case for those; a case +asserting them would encode parser behavior the rule does not depend on. + +### 5. DoS (billion-laughs / recursive anchors) + +A recursive YAML bomb needs both an anchor definition and an alias that references it; a +single scalar value cannot form that structure in a way that survives rendering, and a +bare `*alias` errors fatally (see §1). Not meaningfully coverable at the value level. diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/expected.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/expected.yaml new file mode 100644 index 00000000..6f0601c1 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/expected.yaml @@ -0,0 +1,74 @@ +description: > + Broad YAML-injection coverage for the openapi-values-quote rule, beyond the `\n---\n` + document separator. Each unconstrained string default is a different hazard that a + bare (unquoted) render turns real: `\n...\n` truncates the document; an indented + `\n key: val` injects a sibling key into the mapping; `no` / `0755` / `~` are silently + coerced to bool / octal-int / null; ` # …` truncates the value as a comment; a leading + `[` or `{` turns the string into a list or map; a leading `!` or `&` is eaten as a tag + or anchor. All of these render to VALID YAML, so the module loads and the rule can be + asserted: every unquoted usage must be flagged, and the same value must be silent when + neutralised with `| quote` or constrained by a `pattern`. Payloads that instead make + the document unparseable (bare `key: value`, leading `@`, `*undefined`, a column-0 key) + are a documented e2e limitation (see README.md): dmt aborts module creation on them, so + the rule finding cannot be observed here. +module: module +expect: + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eYamlHazards.docEnd" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eYamlHazards.keyInject" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eYamlHazards.coerceBool" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eYamlHazards.coerceOctal" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eYamlHazards.coerceNull" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eYamlHazards.truncHash" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eYamlHazards.flowSeq" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eYamlHazards.flowMap" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eYamlHazards.tagBang" + count: 1 + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eYamlHazards.anchorAmp" + count: 1 +expectAbsent: + # dangerous value used only through `| quote` -> not reported + - linter: templates + rule: openapi-values-quote + textContains: "e2eYamlHazards.safeQuoted" + # pattern-constrained value -> not reported + - linter: templates + rule: openapi-values-quote + textContains: "e2eYamlHazards.safePattern" diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/module.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/module.yaml new file mode 100644 index 00000000..19461196 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/module.yaml @@ -0,0 +1,2 @@ +name: e2e-yaml-hazards +namespace: e2e-yaml-hazards diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/openapi/config-values.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/openapi/config-values.yaml new file mode 100644 index 00000000..4402e05b --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/openapi/config-values.yaml @@ -0,0 +1,59 @@ +type: object +# Each property below is an unconstrained OpenAPI string (no pattern/enum/format) whose +# default is a different YAML-injection payload. Rendered UNQUOTED every one of them is a +# hazard; the openapi-values-quote rule must flag each, and stay silent when it is quoted +# or pattern-constrained. The payloads here all render to VALID YAML (so the module loads +# and the rule can be asserted) — the harm is a wrong structure or a silently changed +# type, verified against dmt's real render. Payloads that instead make the document +# unparseable are documented in README.md as a known e2e limitation, not covered here. +properties: + # STRUCTURAL: `\n...\n` document-end marker truncates the rest of the document. + docEnd: + type: string + default: "legit\n...\n" + # STRUCTURAL: `\n key: val` (indented to match the field) injects a sibling key into + # the current mapping — no new document needed. Verified: adds `injected: pwned`. + keyInject: + type: string + default: "legit\n injected: pwned" + # TYPE COERCION: `no` parses as boolean false (the "Norway problem"). + coerceBool: + type: string + default: "no" + # TYPE COERCION: `0755` parses as an octal integer (493). + coerceOctal: + type: string + default: "0755" + # TYPE COERCION: `~` parses as null. + coerceNull: + type: string + default: "~" + # TRUNCATION: everything after ` #` is treated as a comment and silently dropped. + truncHash: + type: string + default: "visible # hidden-secret" + # TYPE CHANGE: a leading `[` starts a flow sequence -> the string becomes a list. + flowSeq: + type: string + default: "[a, b]" + # TYPE CHANGE: a leading `{` starts a flow mapping -> the string becomes a map. + flowMap: + type: string + default: "{a: b}" + # INDICATOR: a leading `!` is read as a YAML tag; verified the value renders empty. + tagBang: + type: string + default: "!badtag" + # INDICATOR: a leading `&` is read as an anchor definition; verified it is stripped. + anchorAmp: + type: string + default: "&anchor kept" + # SAFE (mitigated at use site): a dangerous value used only through `| quote`. + safeQuoted: + type: string + default: "legit\n---\nkind: Secret" + # SAFE (constrained by schema): a pattern that forbids every hazardous character. + safePattern: + type: string + pattern: '^[a-z]+$' + default: safe diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/openapi/values.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/openapi/values.yaml new file mode 100644 index 00000000..47180da5 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/openapi/values.yaml @@ -0,0 +1,4 @@ +x-extend: + schema: config-values.yaml +type: object +properties: {} diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/haz-coercion.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/haz-coercion.yaml new file mode 100644 index 00000000..00363a32 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/haz-coercion.yaml @@ -0,0 +1,12 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-yaml-hazards-coercion + namespace: e2e-yaml-hazards +data: + # FLAGGED: unquoted string that YAML silently reads as a boolean (`no` -> false). + asBool: {{ .Values.e2eYamlHazards.coerceBool }} + # FLAGGED: unquoted string read as an octal integer (`0755` -> 493). + asOctal: {{ .Values.e2eYamlHazards.coerceOctal }} + # FLAGGED: unquoted string read as null (`~`). + asNull: {{ .Values.e2eYamlHazards.coerceNull }} diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/haz-doc-end.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/haz-doc-end.yaml new file mode 100644 index 00000000..7fde8932 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/haz-doc-end.yaml @@ -0,0 +1,10 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-yaml-hazards-doc-end + namespace: e2e-yaml-hazards +data: + # FLAGGED: unquoted value carrying `\n...\n` -> the doc-end marker truncates everything + # below it (the `dropped` field silently disappears from the rendered manifest). + field: {{ .Values.e2eYamlHazards.docEnd }} + dropped: this-field-is-truncated-away diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/haz-flow.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/haz-flow.yaml new file mode 100644 index 00000000..84dae39f --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/haz-flow.yaml @@ -0,0 +1,11 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-yaml-hazards-flow + namespace: e2e-yaml-hazards +data: + # FLAGGED: unquoted value beginning with `[` -> parsed as a flow sequence (list), + # changing the value's type from string to array. + asSeq: {{ .Values.e2eYamlHazards.flowSeq }} + # FLAGGED: unquoted value beginning with `{` -> parsed as a flow mapping (object). + asMap: {{ .Values.e2eYamlHazards.flowMap }} diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/haz-indicators.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/haz-indicators.yaml new file mode 100644 index 00000000..94ce2252 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/haz-indicators.yaml @@ -0,0 +1,12 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-yaml-hazards-indicators + namespace: e2e-yaml-hazards +data: + # FLAGGED: unquoted value beginning with `!` -> read as a YAML tag; the intended text + # is lost (verified: renders as an empty value). + asTag: {{ .Values.e2eYamlHazards.tagBang }} + # FLAGGED: unquoted value beginning with `&` -> read as an anchor definition; the `&` + # token is stripped from the stored value. + asAnchor: {{ .Values.e2eYamlHazards.anchorAmp }} diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/haz-key-injection.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/haz-key-injection.yaml new file mode 100644 index 00000000..560f9a97 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/haz-key-injection.yaml @@ -0,0 +1,9 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-yaml-hazards-key-injection + namespace: e2e-yaml-hazards +data: + # FLAGGED: unquoted value whose payload is indented to match this field, injecting a + # sibling key (`injected: pwned`) into `data` without needing a new document. + field: {{ .Values.e2eYamlHazards.keyInject }} diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/haz-truncation.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/haz-truncation.yaml new file mode 100644 index 00000000..b39c118c --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/haz-truncation.yaml @@ -0,0 +1,9 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-yaml-hazards-truncation + namespace: e2e-yaml-hazards +data: + # FLAGGED: unquoted value containing ` #` -> the part after it is parsed as a YAML + # comment and silently dropped (verified: only `visible` survives). + field: {{ .Values.e2eYamlHazards.truncHash }} diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/safe.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/safe.yaml new file mode 100644 index 00000000..2351bf8e --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-hazards/module/templates/safe.yaml @@ -0,0 +1,12 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-yaml-hazards-safe + namespace: e2e-yaml-hazards +data: + # SAFE: a dangerous value neutralised with `| quote` -> stays a single string scalar. + quoted: {{ .Values.e2eYamlHazards.safeQuoted | quote }} + # SAFE: the boolean-looking value, quoted -> stays the string "no". + boolQuoted: {{ .Values.e2eYamlHazards.coerceBool | quote }} + # SAFE: constrained by a pattern that forbids hazardous characters -> quoting optional. + pattern: {{ .Values.e2eYamlHazards.safePattern }} diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-injection-safe/expected.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection-safe/expected.yaml new file mode 100644 index 00000000..03d03365 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection-safe/expected.yaml @@ -0,0 +1,13 @@ +description: > + Negative control for the `\n---\n` YAML document-separator injection scenario. The + module holds the same dangerous unconstrained strings (each default carries a + newline + `---` + newline payload) but every template usage neutralises them — + `| quote` on the scalar, nested field, `with`-scoped value, array elements and the + array-of-objects `host` subfield, plus a `pattern`-constrained value used bare. A + correctly hardened module must produce NO openapi-values-quote findings (no false + positives), and the render must not emit any injected document. +module: module +expectPass: + # the rule must be completely silent on a module that quotes/constrains every value + - linter: templates + rule: openapi-values-quote diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-injection-safe/module/module.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection-safe/module/module.yaml new file mode 100644 index 00000000..e38149be --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection-safe/module/module.yaml @@ -0,0 +1,2 @@ +name: e2e-yaml-injection-safe +namespace: e2e-yaml-injection-safe diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-injection-safe/module/openapi/config-values.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection-safe/module/openapi/config-values.yaml new file mode 100644 index 00000000..dfe63748 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection-safe/module/openapi/config-values.yaml @@ -0,0 +1,81 @@ +type: object +# Every string below is unconstrained (no pattern/enum/format) and carries a dangerous +# `\n---\n` document-separator payload, EXCEPT `safeCode`, which a `pattern` constrains. +# The templates neutralise all of them, so the openapi-values-quote rule must stay silent. +properties: + # unconstrained string with a `\n---\n` payload + scalar: + type: string + default: |- + legit-value + --- + apiVersion: v1 + kind: Secret + metadata: + name: would-be-injected + namespace: e2e-yaml-injection-safe + stringData: + pwned: "true" + # array of unconstrained strings, one element carrying a `\n---\n` payload + args: + type: array + default: + - "--safe-flag" + - |- + --flag + --- + apiVersion: v1 + kind: Secret + metadata: + name: would-be-injected-arg + namespace: e2e-yaml-injection-safe + items: + type: string + # nested object whose unconstrained string leaf carries a `\n---\n` payload + conn: + type: object + default: {} + properties: + dsn: + type: string + default: |- + host=db + --- + apiVersion: v1 + kind: Secret + metadata: + name: would-be-injected-nested + namespace: e2e-yaml-injection-safe + # `with`-scoped unconstrained string with a `\n---\n` payload + withScalar: + type: string + default: |- + legit-value + --- + apiVersion: v1 + kind: Secret + metadata: + name: would-be-injected-with + namespace: e2e-yaml-injection-safe + # array of objects whose `host` subfield is an unconstrained string with a payload + servers: + type: array + default: + - host: |- + legit-host + --- + apiVersion: v1 + kind: Secret + metadata: + name: would-be-injected-server + namespace: e2e-yaml-injection-safe + items: + type: object + properties: + host: + type: string + # SAFE by schema: a pattern that forbids newlines and `---` + safeCode: + type: string + pattern: '^[a-zA-Z0-9]+$' + default: safecode diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-injection-safe/module/openapi/values.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection-safe/module/openapi/values.yaml new file mode 100644 index 00000000..47180da5 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection-safe/module/openapi/values.yaml @@ -0,0 +1,4 @@ +x-extend: + schema: config-values.yaml +type: object +properties: {} diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-injection-safe/module/templates/configmap.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection-safe/module/templates/configmap.yaml new file mode 100644 index 00000000..c9ae4836 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection-safe/module/templates/configmap.yaml @@ -0,0 +1,26 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-yaml-injection-safe + namespace: e2e-yaml-injection-safe +data: + # SAFE: `| quote` escapes the `\n---\n`, so it stays inside the scalar. + scalar: {{ .Values.e2eYamlInjectionSafe.scalar | quote }} + # SAFE: nested string leaf, quoted. + dsn: {{ .Values.e2eYamlInjectionSafe.conn.dsn | quote }} + # SAFE: constrained by a pattern -> injection impossible per schema, quoting optional. + code: {{ .Values.e2eYamlInjectionSafe.safeCode }} +{{- with .Values.e2eYamlInjectionSafe.withScalar }} + # SAFE: `with`-scoped scalar, quoted. + scoped: {{ . | quote }} +{{- end }} +# SAFE: every array element quoted inside the range. +args: +{{- range .Values.e2eYamlInjectionSafe.args }} + - {{ . | quote }} +{{- end }} +# SAFE: `host` subfield of each array-of-objects element, quoted. +hosts: +{{- range $s := .Values.e2eYamlInjectionSafe.servers }} + - host: {{ $s.host | quote }} +{{- end }} diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/expected.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/expected.yaml new file mode 100644 index 00000000..332e2eeb --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/expected.yaml @@ -0,0 +1,60 @@ +description: > + YAML document-separator injection (`\n---\n`) coverage for the openapi-values-quote + rule. An unconstrained OpenAPI string (no pattern/enum/format) that ends up holding a + newline + `---` + newline breaks out of its scalar when rendered UNQUOTED and injects a + whole rogue YAML document (e.g. a Secret) into the manifest stream. Because module + values are unconstrained, a template can never assume they are injection-safe, so the + rule must flag every unquoted injection surface — scalar, array element, map value, + nested object field, `with`-scoped scalar, and array-of-objects subfield — and must + stay silent when the value is neutralised with `| quote` (which escapes the newlines) + or constrained by a `pattern` that makes `\n---\n` unrepresentable. The dangerous + values are given `\n---\n` defaults so the threat is concrete: rendered with dmt's + synthetic values these unquoted usages actually emit extra `Secret` documents into the + object store (verifiable at render), whereas every quoted usage does not. +module: module +expect: + # scalar rendered unquoted -> the `\n---\n` in its value injects a Secret document + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eYamlInjection.injectedScalar" + count: 1 + # unquoted element of a string array -> a `\n---\n` element injects a document + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eYamlInjection.injectedArgs" + count: 1 + # unquoted value of a string map -> a `\n---\n` value injects a document + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eYamlInjection.injectedAnnotations" + count: 1 + # unquoted nested object field -> a `\n---\n` value injects a document + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eYamlInjection.injectedConn.dsn" + count: 1 + # unconstrained string scoped by `with`, rendered unquoted -> injection + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eYamlInjection.withScalar" + count: 1 + # unquoted `host` subfield of an array-of-objects element -> injection + - linter: templates + rule: openapi-values-quote + level: error + textContains: "e2eYamlInjection.servers[].host" + count: 1 +expectAbsent: + # the same dangerous value used only through quote is not injectable -> not reported + - linter: templates + rule: openapi-values-quote + textContains: "e2eYamlInjection.mitigatedScalar" + # a pattern that forbids newlines/`---` makes injection impossible -> not reported + - linter: templates + rule: openapi-values-quote + textContains: "e2eYamlInjection.safeCode" diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/module.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/module.yaml new file mode 100644 index 00000000..1b7965ac --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/module.yaml @@ -0,0 +1,2 @@ +name: e2e-yaml-injection +namespace: e2e-yaml-injection diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/openapi/config-values.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/openapi/config-values.yaml new file mode 100644 index 00000000..b9d8a2c1 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/openapi/config-values.yaml @@ -0,0 +1,123 @@ +type: object +properties: + # FLAGGED: an unconstrained string (no pattern/enum/format). The default carries a + # newline + `---` + newline payload: rendered UNQUOTED it breaks out of its scalar + # and injects a whole rogue YAML document (a Secret) into the manifest stream. + injectedScalar: + type: string + default: |- + legit-value + --- + apiVersion: v1 + kind: Secret + metadata: + name: injected-by-scalar + namespace: e2e-yaml-injection + stringData: + pwned: "true" + # FLAGGED: array of unconstrained strings; one element carries the `\n---\n` payload, + # so an unquoted `- {{ . }}` inside a range injects a document. + injectedArgs: + type: array + default: + - "--safe-flag" + - |- + --injected-flag + --- + apiVersion: v1 + kind: Secret + metadata: + name: injected-by-arg + namespace: e2e-yaml-injection + stringData: + pwned: "true" + items: + type: string + # FLAGGED: map of unconstrained strings. An unquoted map value inside a range is the + # same injection surface; the rule flags it statically (a real `\n---\n` map value + # would break out into a new document). + injectedAnnotations: + type: object + default: + note: |- + ok + --- + apiVersion: v1 + kind: ConfigMap + metadata: + name: injected-by-map + namespace: e2e-yaml-injection + data: + pwned: "yes" + additionalProperties: + type: string + # FLAGGED: nested object whose unconstrained string leaf carries the `\n---\n` payload + # (the default lives on the leaf so dmt's value composition renders it into the field). + injectedConn: + type: object + default: {} + properties: + dsn: + type: string + default: |- + host=db + --- + apiVersion: v1 + kind: Secret + metadata: + name: injected-by-nested + namespace: e2e-yaml-injection + stringData: + pwned: "true" + # FLAGGED: unconstrained string reached through a `with` block; its `\n---\n` default + # injects a document when the scoped `{{ . }}` is rendered unquoted. + withScalar: + type: string + default: |- + legit-value + --- + apiVersion: v1 + kind: Secret + metadata: + name: injected-by-with + namespace: e2e-yaml-injection + stringData: + pwned: "true" + # FLAGGED: array of objects; the `host` subfield is an unconstrained string carrying + # the `\n---\n` payload, injected when rendered unquoted inside a named range. + servers: + type: array + default: + - host: |- + legit-host + --- + apiVersion: v1 + kind: Secret + metadata: + name: injected-by-server + namespace: e2e-yaml-injection + stringData: + pwned: "true" + items: + type: object + properties: + host: + type: string + # SAFE (mitigated at use site): an unconstrained string with the same dangerous + # payload, but every template usage quotes it, so `\n---\n` stays inside the scalar. + mitigatedScalar: + type: string + default: |- + legit-value + --- + apiVersion: v1 + kind: Secret + metadata: + name: never-injected + # SAFE (constrained at the schema): a `pattern` forbids newlines and `-`-only tokens, + # so a `\n---\n` value can never validate — the injection is impossible by contract + # and the template does not need to quote it. + safeCode: + type: string + pattern: '^[a-zA-Z0-9]+$' + default: safecode diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/openapi/values.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/openapi/values.yaml new file mode 100644 index 00000000..47180da5 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/openapi/values.yaml @@ -0,0 +1,4 @@ +x-extend: + schema: config-values.yaml +type: object +properties: {} diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/safe.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/safe.yaml new file mode 100644 index 00000000..e05bc51b --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/safe.yaml @@ -0,0 +1,21 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-yaml-injection-safe + namespace: e2e-yaml-injection +data: + # SAFE: the same dangerous value, piped through quote -> the `\n---\n` payload is + # escaped and stays inside the scalar, so no rogue document is injected. For a + # multi-line value only `| quote` is a correct fix; a raw double-quote wrap would + # embed real newlines and produce invalid YAML. + scalarQuoted: {{ .Values.e2eYamlInjection.injectedScalar | quote }} + # SAFE: a value used only through quote -> never injectable. + mitigated: {{ .Values.e2eYamlInjection.mitigatedScalar | quote }} + # SAFE: constrained by a pattern that forbids newlines and `---`, so injection is + # impossible per schema and quoting is not required. + code: {{ .Values.e2eYamlInjection.safeCode }} +# SAFE: every array element quoted inside the range. +safeArgs: +{{- range .Values.e2eYamlInjection.injectedArgs }} + - {{ . | quote }} +{{- end }} diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/vuln-list.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/vuln-list.yaml new file mode 100644 index 00000000..9780f203 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/vuln-list.yaml @@ -0,0 +1,11 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-yaml-injection-list + namespace: e2e-yaml-injection +# FLAGGED: unquoted element of an unconstrained string array. The element whose value +# carries `\n---\n` breaks out of the list and injects the Secret from its default. +extraArgs: +{{- range .Values.e2eYamlInjection.injectedArgs }} + - {{ . }} +{{- end }} diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/vuln-map.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/vuln-map.yaml new file mode 100644 index 00000000..dc36889c --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/vuln-map.yaml @@ -0,0 +1,10 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-yaml-injection-map + namespace: e2e-yaml-injection +data: +{{- range $k, $v := .Values.e2eYamlInjection.injectedAnnotations }} + # FLAGGED: unquoted map value -> a `\n---\n` value here would inject a document. + {{ $k }}: {{ $v }} +{{- end }} diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/vuln-nested.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/vuln-nested.yaml new file mode 100644 index 00000000..10a524e4 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/vuln-nested.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-yaml-injection-nested + namespace: e2e-yaml-injection +data: + # FLAGGED: unconstrained nested string field rendered unquoted -> `\n---\n` injection. + dsn: {{ .Values.e2eYamlInjection.injectedConn.dsn }} diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/vuln-scalar.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/vuln-scalar.yaml new file mode 100644 index 00000000..2a1fee94 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/vuln-scalar.yaml @@ -0,0 +1,9 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-yaml-injection-scalar + namespace: e2e-yaml-injection +data: + # FLAGGED: unconstrained string rendered unquoted. The value's embedded `\n---\n` + # breaks out of the scalar and injects the Secret document carried in its default. + scalar: {{ .Values.e2eYamlInjection.injectedScalar }} diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/vuln-servers.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/vuln-servers.yaml new file mode 100644 index 00000000..ab457298 --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/vuln-servers.yaml @@ -0,0 +1,11 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-yaml-injection-servers + namespace: e2e-yaml-injection +# FLAGGED: unquoted `host` subfield of an array-of-objects element. The element whose +# host carries `\n---\n` breaks out and injects the Secret from its default. +hosts: +{{- range $s := .Values.e2eYamlInjection.servers }} + - host: {{ $s.host }} +{{- end }} diff --git a/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/vuln-with.yaml b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/vuln-with.yaml new file mode 100644 index 00000000..4396883c --- /dev/null +++ b/test/e2e/testdata/templates/openapi-values-quote-yaml-injection/module/templates/vuln-with.yaml @@ -0,0 +1,11 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: e2e-yaml-injection-with + namespace: e2e-yaml-injection +data: +{{- with .Values.e2eYamlInjection.withScalar }} + # FLAGGED: unconstrained string scoped by `with`, rendered unquoted -> `\n---\n` + # in the value injects a document. + scoped: {{ . }} +{{- end }} From 36b78aa37952b24d4bbbff0f46d76cf638054418 Mon Sep 17 00:00:00 2001 From: Pavel Okhlopkov Date: Thu, 20 Aug 2026 20:08:05 +0300 Subject: [PATCH 6/8] [dmt] exempt nindent/indent/fail from openapi-values-quote Signed-off-by: Pavel Okhlopkov --- .../templates/rules/openapi_values_quote.go | 23 +++++++-- .../rules/openapi_values_quote_test.go | 48 +++++++++++++++++++ 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/pkg/linters/templates/rules/openapi_values_quote.go b/pkg/linters/templates/rules/openapi_values_quote.go index eb67505d..89048e4b 100644 --- a/pkg/linters/templates/rules/openapi_values_quote.go +++ b/pkg/linters/templates/rules/openapi_values_quote.go @@ -51,6 +51,17 @@ var safeFuncs = map[string]struct{}{ "sha1sum": {}, "sha256sum": {}, "sha512sum": {}, "adler32sum": {}, } +// blockOrSinkFuncs are pipeline functions after which quoting is impossible or +// meaningless, so a value routed through one of them is exempt from this rule: +// - nindent/indent place the value as an indented (multi-line) block — e.g. a CA +// bundle under a `key: |` block scalar, or a base64 blob expanded via `b64dec`; +// wrapping such output in quotes would corrupt it, so "must be quoted" never applies. +// - fail consumes the value into an aborting error message that is never rendered +// into the manifest, so it cannot be a YAML-injection vector. +var blockOrSinkFuncs = map[string]struct{}{ + "nindent": {}, "indent": {}, "fail": {}, +} + // templateActionRe matches a single Go-template action {{ ... }}, capturing the // inner expression with the surrounding whitespace/`-` trim markers stripped. var templateActionRe = regexp.MustCompile(`{{-?\s*(.*?)\s*-?}}`) @@ -1192,12 +1203,18 @@ func isParenWrapped(s string) bool { return false } -// pipelineIsSafe reports whether any function stage of the pipeline is known to emit -// a YAML-safe scalar (see safeFuncs). +// pipelineIsSafe reports whether the pipeline routes the value through a stage that +// makes an explicit quote unnecessary — either because the stage emits a YAML-safe +// scalar (see safeFuncs) or because the value is placed as a block / never emitted +// (see blockOrSinkFuncs), in which case quoting is impossible or meaningless. func pipelineIsSafe(inner string) bool { stages := splitPipeline(inner) for _, stage := range stages[1:] { - if _, ok := safeFuncs[firstToken(stage)]; ok { + tok := firstToken(stage) + if _, ok := safeFuncs[tok]; ok { + return true + } + if _, ok := blockOrSinkFuncs[tok]; ok { return true } } diff --git a/pkg/linters/templates/rules/openapi_values_quote_test.go b/pkg/linters/templates/rules/openapi_values_quote_test.go index ed24e349..88ac7128 100644 --- a/pkg/linters/templates/rules/openapi_values_quote_test.go +++ b/pkg/linters/templates/rules/openapi_values_quote_test.go @@ -96,6 +96,54 @@ properties: }, wantCount: 0, }, + { + name: "value placed as a block via nindent is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ .Values." + valuesKey + ".foo | nindent 4 }}\n", + }, + wantCount: 0, + }, + { + name: "value placed as a block via indent is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n foo: {{ .Values." + valuesKey + ".foo | indent 4 }}\n", + }, + wantCount: 0, + }, + { + name: "base64 value expanded via b64dec|nindent is not flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n{{ .Values." + valuesKey + ".foo | b64dec | nindent 2 }}\n", + }, + wantCount: 0, + }, + { + name: "value consumed by fail is never emitted, not flagged", + valuesSchema: `type: object +properties: + foo: + type: string +`, + files: map[string]string{ + "templates/cm.yaml": "data:\n {{ cat \"missing key:\" .Values." + valuesKey + ".foo | fail }}\n", + }, + wantCount: 0, + }, { name: "string with pattern is not flagged", valuesSchema: `type: object From df6427a8aa11536c54db8c684804c8e606f3b8c9 Mon Sep 17 00:00:00 2001 From: Pavel Okhlopkov Date: Thu, 20 Aug 2026 21:01:50 +0300 Subject: [PATCH 7/8] fix lint Signed-off-by: Pavel Okhlopkov --- pkg/linters/templates/rules/openapi_values_quote.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/linters/templates/rules/openapi_values_quote.go b/pkg/linters/templates/rules/openapi_values_quote.go index 89048e4b..91042050 100644 --- a/pkg/linters/templates/rules/openapi_values_quote.go +++ b/pkg/linters/templates/rules/openapi_values_quote.go @@ -1214,6 +1214,7 @@ func pipelineIsSafe(inner string) bool { if _, ok := safeFuncs[tok]; ok { return true } + if _, ok := blockOrSinkFuncs[tok]; ok { return true } From 0a993aa43c9ea44d8675efe4664006de16f73e17 Mon Sep 17 00:00:00 2001 From: Pavel Okhlopkov Date: Thu, 20 Aug 2026 21:19:32 +0300 Subject: [PATCH 8/8] fix Signed-off-by: Pavel Okhlopkov --- .../templates/rules/openapi_values_quote.go | 40 ++++++++++++++----- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/pkg/linters/templates/rules/openapi_values_quote.go b/pkg/linters/templates/rules/openapi_values_quote.go index 91042050..e3e470e1 100644 --- a/pkg/linters/templates/rules/openapi_values_quote.go +++ b/pkg/linters/templates/rules/openapi_values_quote.go @@ -1032,7 +1032,7 @@ func (r *OpenAPIValuesQuoteRule) checkInclude( emits map[string]defineEmit, errorList *errors.LintRuleErrorsList, ) { emit, ok := emits[name] - if !ok || pipelineIsSafe(inner) { + if !ok || pipelineHasSafeFunc(inner) { return } @@ -1203,19 +1203,37 @@ func isParenWrapped(s string) bool { return false } -// pipelineIsSafe reports whether the pipeline routes the value through a stage that -// makes an explicit quote unnecessary — either because the stage emits a YAML-safe -// scalar (see safeFuncs) or because the value is placed as a block / never emitted -// (see blockOrSinkFuncs), in which case quoting is impossible or meaningless. -func pipelineIsSafe(inner string) bool { - stages := splitPipeline(inner) - for _, stage := range stages[1:] { - tok := firstToken(stage) - if _, ok := safeFuncs[tok]; ok { +// pipelineHasSafeFunc reports whether any stage of the pipeline emits a YAML-safe +// scalar (see safeFuncs) — i.e. the value's own rendered output is quoted or encoded. +// This is the check to use when a value is emitted transitively (e.g. inside an +// included template), where only real quoting of the output — not block re-indenting — +// can protect it. +func pipelineHasSafeFunc(inner string) bool { + for _, stage := range splitPipeline(inner)[1:] { + if _, ok := safeFuncs[firstToken(stage)]; ok { return true } + } + + return false +} + +// pipelineIsSafe reports whether the value emitted directly by this action needs no +// explicit quote — either because a stage emits a YAML-safe scalar (pipelineHasSafeFunc) +// or because the value is placed as a block / never emitted (see blockOrSinkFuncs), in +// which case quoting is impossible or meaningless. +// +// blockOrSinkFuncs only exempts a value flowing directly into the pipeline. For an +// include/template argument the value is emitted inside the callee (see checkInclude), +// where an outer nindent/indent re-indents but does not quote it, so that path uses +// pipelineHasSafeFunc instead. +func pipelineIsSafe(inner string) bool { + if pipelineHasSafeFunc(inner) { + return true + } - if _, ok := blockOrSinkFuncs[tok]; ok { + for _, stage := range splitPipeline(inner)[1:] { + if _, ok := blockOrSinkFuncs[firstToken(stage)]; ok { return true } }