fix(flags): reject an empty element in a multi-value flag

[JonJagger]

pflag's StringSlice splits each value with a CSV reader, and that reader yields
nothing for an empty string. An empty element is therefore appended as nothing
and leaves no trace: after parsing, --attachments "" --attachments file cannot
be told apart from --attachments file, so nothing downstream can report what
was lost.

Two flags where that matters now use a value type that refuses an empty element
at Set, the last point where it still exists. --attachments is the case this bug
was filed for: an attestation is recorded without evidence its author believed
was on it. --template is worse in kind, because it names the attestations a flow
requires, so a dropped element weakens that flow for every artifact passing
through it afterwards rather than spoiling one record.

Comma splitting is kept rather than switching to pflag's StringArray, which
stores values verbatim. An environment variable cannot be repeated, so a comma
list is the only way to give a multi-value flag more than one value from the
environment; a regression test pins that.

The type implements pflag.SliceValue as well as pflag.Value, so the refusal also
covers the config file, which bindFlags applies through Replace.

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 2m 2s —— View job

---

Review — all prior feedback addressed ✅

Re-reviewed after the latest push (2bfd5218, 9bc21f3e). The earlier review threads are now resolved and the change is in good shape.

Static checks

• go build ./... — clean
• go vet ./cmd/kosli/ — clean

Prior review points, now resolved

• The doc comment in nonEmptyStringSlice.go:18-21 no longer claims config/env values arrive via Replace. It correctly states they flow through Set (stringified with %v), so the refusal covers them by the same path as argv. This matches bindFlags at root.go:641.
• The comment now also honestly frames Append/Replace/GetSlice and the pflag.SliceValue assertion as future-proofing (:11-16), rather than load-bearing config-path code.
• The "config/env only logs and continues" concern is genuinely closed: bindFlags joins the failed Set into bindErr (root.go:642) and initializeConfig returns it (root.go:544-546), so KOSLI_ATTACHMENTS="a,,b" is a hard failure, not a silent drop.

What's good here

• The fix targets the right layer — Set is the last point where the empty element still exists, and refusing there is the minimal correct intervention. The extensive comments explain why (CSV reader eats empty strings) rather than what.
• Test coverage is thorough and well-motivated: table-driven rejection cases ("", a,,b, ,a, a,), the comma-splitting regression guard that justifies not switching to StringArray, and — new in this push — end-to-end argv and env-path tests for both --attachments and --template. Using a real file (testdata/file1) as the surviving attachment so the test can't pass for the wrong reason is a nice touch.
• String()/Type() deliberately mirror pflag's StringSlice so help text and defaults render identically after the flag switches type — no doc regression.

Minor, optional (non-blocking)

• The literal "empty values are not allowed" appears in three places (Set, Append, Replace). A shared errEmptyElement var would keep them from drifting, but it's harmless as-is.
• Replace assigns the caller's slice directly (*s.values = values) rather than copying, unlike pflag's StringSlice. No caller exercises it today, so this is theoretical — worth a note only if Replace ever becomes reachable.

Nothing blocking. LGTM.

• Read changed files
• Run static checks (build, vet)
• Verify earlier review feedback is addressed
• Post review