feat(config): read YAML lists and maps, and refuse shapes that do not…

[JonJagger]

… fit

A config file is YAML, so a multi-value flag invites a YAML list and a key=value
flag invites a YAML mapping. Neither worked. Values were applied by rendering
them with %v, which turns a list into "[coverage unit-test]" and a mapping into
"map[docs:https://...]", and the flag stored the rendering whole. A flow created
that way required an attestation named "[coverage unit-test]", which nothing can
ever satisfy, and the command exited 0.

Nothing told the user to write a comma-separated string instead. That rule
appears only in kosli config --help, describing the --set flag it writes with,
not the contents of the file people hand-edit.

Both spellings are now read: a list onto any multi-value flag, a mapping onto a
key=value flag. The comma-separated forms are untouched.

The rest of the change is the default. Rendering with %v turns any shape into a
plausible-looking string, so anything unrecognised used to become quiet
nonsense - a list of mappings would have put "map[name:coverage]" in a template.
A decoded value is a sequence, a mapping or a scalar, so refusing what does not
fit is a closed rule rather than a list of known mistakes: a sequence needs a
multi-value flag and scalar elements, a mapping needs a key=value flag and
scalar values, and anything else names the flag and what it expected.

Two fixtures changed because their unappliable value became appliable: a YAML
mapping is now valid, so they use a scalar without an "=" instead.

Checklist

• Docs are autogenerated from CLI help — make sure the help text reflects your changes
• Helm chart (charts/k8s-reporter/) updated, if needed. Note: these changes live in a separate PR
• Terraform provider and related changes (terraform-provider-kosli, terraform-aws-evidence-reporter, terraform-aws-kosli-reporter) updated, if needed

[claude]

Claude finished @JonJagger's task in 3m 11s —— View job

---

PR Review: read YAML lists and maps in config files

• Static analysis (go build ./..., go vet ./cmd/kosli/) — both pass ✅
• Review root.go changes
• Review tests & fixtures
• Post feedback

---

This is a strong, thoughtful PR. The core idea is right: a decoded YAML value is a sequence, a mapping, or a scalar, and framing the check as "does this shape fit the flag?" (a closed rule) rather than "is this one of the known bad shapes?" is the correct design. The comments explaining why are excellent, the tests pin real user-facing behaviour (not just the helper functions), and routing empty-element rejection through the flag's own Replace/Set so the config path meets the CLI path is a nice touch.

Static analysis is clean. No blocking issues — the notes below are all low-severity edge cases.

Findings (all low / edge)

1. A non-scalar api-token still panics, bypassing the new refusal logic. applyConfigValue cleanly refuses lists/maps, but the api-token decryption path in bindFlags runs before it and does an unchecked val.(string) (root.go:721). With:

api-token:
  - foo

val is a []interface{}, so val.(string) panics at runtime (reached whenever the encryption key is retrievable from the credentials store) instead of yielding the clean "expects a single value, but a list was given" error the rest of the change delivers. Pre-existing, but the PR's whole theme is non-scalar config values should be refused, not mishandled — so this is the one remaining hole. A s, ok := val.(string); if !ok { … } guard would close it. Fix this →

2. List and map paths have different quoting rules (inline comment on root.go:638). Lists apply via SliceValue.Replace (no re-parse); maps route each pair back through flags.Set, which re-parses through pflag's stringToString CSV handling. A mapping value containing both = and , can be mis-split. Edge case, but the asymmetry is a small surprise. The same loop also applies earlier pairs before a later one fails validation (partial mutation) — harmless since the command aborts, but not all-or-nothing like the list path.

3. A null list element renders as the literal "<nil>". - "" (explicit empty string) is correctly rejected by the flag's Replace, and there's a test for it. But a bare - (which YAML decodes to nil) flows through fmt.Sprintf("%v", nil) → "<nil>" and is silently accepted — exactly the "plausible-looking nonsense" the PR sets out to prevent. Minor, but worth a nil-guard in scalarString and a fixture row if you want the refusal to be complete.

Nice work

• Closed-rule framing over an enumeration of mistakes.
• Tests exercise real commands (attach-policy, attest generic, create flow) and assert the parsed flag values / error messages, not internals.
• Error messages name the offending flag and the source (config file vs env var), so users know where to look.

[claude]

Minor asymmetry worth noting: the list path applies elements directly via SliceValue.Replace (no re-parsing), but the map path routes each pair back through flags.Set, which re-parses through pflag's stringToString CSV handling. So a mapping value that contains both an = and a , (e.g. a URL with a query string like docs: https://x.com/a?b=1,c=2) can be mis-split by pflag's CSV reader — pflag only takes the CSV branch when the pair has 2+ = signs. A single-=, comma-containing value is fine. Very much an edge case, but the two paths having different quoting rules is a small surprise.

Also (nit): in this loop earlier pairs are Set before a later pair fails validation, leaving a partial mutation. Harmless since bindErr aborts the command, but unlike the list path (which validates all elements before Replace) it isn't all-or-nothing.