feat: generalize flag sync to support model flags + generic flags

[maskarb]

Extract FlagSpec type and SyncFlags function from the model-specific SyncModelFlags. Both model manifest (models.json) and a generic flags config (flags.json) produce []FlagSpec and feed into the same sync pipeline. Tags are per-flag — model flags get scope:workspace, generic flags get whatever they specify.

Mount ConfigMaps as directories (not subPath) so kubelet auto-syncs changes without pod restarts.

[github-actions]

Claude Code Review

Summary

This PR generalizes the Unleash flag sync pipeline by extracting FlagSpec/FlagTag types and a SyncFlags function that accepts []FlagSpec instead of a raw model manifest. Both the model manifest and a new generic flags.json config file now produce []FlagSpec that feed into the same sync pipeline. The mount path for the models ConfigMap is also corrected from /config to /config/models across backend, operator, and manifests.

The change is well-executed: clean abstraction, good test coverage, and proper log-injection sanitization at input boundaries. No security regressions found.

Issues by Severity

Blocker Issues

None.

Critical Issues

None.

Major Issues

None.

Minor Issues

1. SyncFlags has no documented sanitization contract — future callers risk log injection

components/backend/cmd/sync_flags.go (SyncFlags, ~lines 166–229)

SyncFlags logs flag.Name, tag.Type, and tag.Value directly into log.Printf calls at multiple points (e.g., " ERROR checking %s: %v", " WARNING: created %s but failed to add tag %s:%s: %v"). Currently all callers sanitize at input time (FlagsFromManifest, FlagsFromConfig), but the function itself carries no doc comment or internal guard stating that callers are expected to pre-sanitize.

Per security-standards.md, log injection prevention is required:

// Prevent log injection with newlines
name = strings.ReplaceAll(name, "\n", "")
name = strings.ReplaceAll(name, "\r", "")

A future caller bypassing FlagsFromConfig/FlagsFromManifest would introduce the vulnerability silently. Suggested fix — add a contract comment to SyncFlags:

// SyncFlags ensures every FlagSpec has a corresponding Unleash feature flag.
// Callers MUST sanitize FlagSpec.Name, Description, and all Tag fields
// (strip \n and \r) before passing to prevent log injection.

Or, more defensively, sanitize inside SyncFlags as well (no double-sanitization cost for already-clean strings).

---

2. Removed per-model exclusion logging reduces startup observability

components/backend/cmd/sync_flags.go (FlagsFromManifest)

The original SyncModelFlags logged each skipped model individually:

log.Printf("  %s: default model, no flag needed", model.ID)
log.Printf("  %s: not available, skipping flag creation", model.ID)

FlagsFromManifest now silently drops them, and the excluded counter in the summary log is gone too. When debugging why a model has no Unleash flag, the startup logs no longer provide any signal.

Suggested: restore at least an aggregate count, or add per-model logging:

log.Printf("  %s: skipped (default or unavailable)", model.ID)

---

3. TestSyncFlags_NoTagsSkipsTagCall — missing UNLEASH_PROJECT / UNLEASH_ENVIRONMENT env vars

components/backend/cmd/sync_flags_test.go (~line 279)

The test doesn't t.Setenv either env var, relying on empty-string defaults being picked up inside SyncFlags. This works today, but if the default-value logic changes, the test will silently start covering different behavior. All sibling tests explicitly set these vars. Suggested fix: add explicit setenv calls for consistency.

Positive Highlights

• Sanitization applied at the right boundary. FlagsFromConfig sanitizes every field of every FlagSpec and FlagTag read from the user-controlled JSON file — exactly matching the pattern in security-standards.md. The dedicated TestFlagsFromConfig_SanitizesNewlines test explicitly validates this.
• Clean abstraction. Separating FlagsFromManifest and FlagsFromConfig as pure producers of []FlagSpec makes SyncFlags genuinely reusable and decouples flag-source logic from sync mechanics.
• doRequest parameter rename (url → reqURL) avoids shadowing the net/url import — good hygiene that prevents subtle bugs.
• flags-config volume is optional: true in the deployment manifest, so existing clusters without the ambient-flags ConfigMap continue working without modification.
• Consistent error wrapping throughout with fmt.Errorf("context: %w", err), matching .claude/patterns/error-handling.md.
• Comprehensive test coverage for new functions (FlagsFromManifest, FlagsFromConfig, collectTagTypes, SyncFlags) including edge cases: empty manifest, missing file, conflict race, and no-tags path.

Recommendations

1. (Minor Outcome: Reduce Refinement Time with agent System #1 — highest priority) Add a doc comment to SyncFlags stating the sanitization pre-condition, or add internal sanitization. This locks the security contract before the API grows more callers.
2. (Minor Epic: RAT Architecture & Design #2) Restore per-model or aggregate exclusion logging in FlagsFromManifest so operators can diagnose missing flags without code inspection.
3. (Minor Epic: Data Source Integration #3) Add explicit t.Setenv("UNLEASH_PROJECT", ...) / t.Setenv("UNLEASH_ENVIRONMENT", ...) to TestSyncFlags_NoTagsSkipsTagCall for consistency with sibling tests.

---

🔍 View AI decision process (logs available for 90 days)

📋 View memory system files loaded (click to expand)

What Amber Loaded for Code Review

Amber automatically loaded these repository standards from the memory system:

1. CLAUDE.md - Master project instructions, development standards
2. backend-development.md - Go backend, K8s integration patterns
3. frontend-development.md - NextJS, Shadcn UI, React Query patterns
4. security-standards.md - Auth, RBAC, token handling
5. k8s-client-usage.md - User token vs service account patterns
6. error-handling.md - Consistent error patterns
7. react-query-usage.md - Data fetching patterns

Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines.