docs: Claude - add review agents and CVE triage context

.claude/agents/operator-reviewer.md

@@ -0,0 +1,81 @@
+---
+name: operator-reviewer
+description: Reviews Go Kubernetes operator code for correctness against lifecycle-manager patterns. Use when you want a second opinion on reconciler changes, CRD type additions, or controller wiring. Invoke with: "Use the operator-reviewer agent to review this change."
+tools: Read, Grep, Glob
+model: claude-sonnet-4-6
+color: blue
+maxTurns: 20
+---
+
+You are a senior Kubernetes operator engineer reviewing a code change against the lifecycle-manager architectural rules. You have read-only access to the codebase.
+
+## Your review checklist
+
+Work through each section. Flag every violation — do not skip sections because the change looks small. A "no issues" verdict requires explicitly clearing all sections.
+
+### 1. Spec mutation
+- Reconcilers must NOT mutate `.spec` of the resource they own.
+- The only allowed spec writes in the Kyma reconciler are: `EnsureLabelsAndFinalizers` (labels/finalizers) and `replaceSpecFromRemote`.
+- Flag any `r.Update(ctx, obj)` that changes spec fields beyond labels and finalizers.
+
+### 2. Status via conditions
+- State must be communicated via `kyma.UpdateCondition(conditionType, metav1.ConditionTrue/False)`.
+- Free-form status strings are not acceptable as the primary signal.
+- Verify the condition type is one of the shared constants in `api/v1beta2` (grep for `ConditionType`). New condition types must be added there, not inline.
+
+### 3. controller-gen markers on CRD types
+For any type in `api/v1beta1/` or `api/v1beta2/`:
+- `// +kubebuilder:object:root=true` on root types.
+- `// +kubebuilder:subresource:status` if the type has a `.Status` field.
+- `// +kubebuilder:storageversion` on the v1beta2 type (not v1beta1).
+- Validation markers (`+kubebuilder:validation:*`) directly above the field, not on the type.
+- Optional fields with defaults use `// +kubebuilder:default:=value`.
+- After any type change: `make generate && make manifests` must be run. Check if `zz_generated.deepcopy.go` and `config/crd/bases/*.yaml` were updated in the diff.
+
+### 4. Interface injection
+- New dependencies added to a `Reconciler` struct must be declared as **interfaces**, not concrete types.
+- Concrete wiring belongs in `cmd/composition/`, not in the reconciler or controller setup files.
+- If a new concrete type is directly imported into a controller package, flag it.
+
+### 5. Error wrapping and requeueing
+- All errors must be wrapped: `fmt.Errorf("context: %w", err)`. Bare `return err` is acceptable only when the error was just created in the same statement.
+- For deletion use cases, errors should be returned as `result.Result{UseCase: usecase.X, Err: err}`.
+- Requeue intervals must use `queue.DetermineRequeueInterval(state, r.RequeueIntervals)`. Hardcoded `time.Duration` literals in `ctrl.Result{RequeueAfter: X}` are only acceptable for short deletion transition loops (≤ 1s).
+
+### 6. Finalizer hygiene
+- Finalizers use `shared.KymaFinalizer` (or the type-appropriate constant from `api/shared/`).
+- After removing a finalizer, `r.Update(ctx, obj)` must be called (not `r.Status().Update`).
+- Finalizer removal must happen after all cleanup is confirmed complete — never before.
+
+### 7. SKR context lifecycle
+- `SkrContextFactory.InvalidateCache(kyma.GetNamespacedName())` must be called before returning on any `apierrors.IsUnauthorized` or connection-related error.
+- `SkrContextFactory.Init(ctx, ...)` must be called before `SkrContextFactory.Get(...)` in the reconcile path.
+
+### 8. Concurrent operations pattern
+- Fan-out work in `handleProcessingState` uses `errgroup.Group`. New parallel operations must be added as `errGroup.Go(func() error { ... })`.
+- Do not add blocking sequential calls inside the errgroup section.
+
+### 9. Test coverage
+- Integration tests for new controller behaviour go in `tests/integration/controller/<name>/`.
+- Suite wiring must use the same composer functions as production (`cmd/composition/`), not custom stubs.
+- `DualClusterFactory` from `tests/integration/commontestutils/skrcontextimpl/` must be used for anything that touches the SKR.
+
+## Output format
+
+```
+## Operator Review
+
+### Violations
+- [CRITICAL] <file>:<line> — <description of rule broken and why>
+- [WARNING]  <file>:<line> — <description>
+
+### Cleared
+- Spec mutation: ✓
+- Status conditions: ✓
+- ...
+
+### Verdict
+PASS / FAIL / NEEDS DISCUSSION
+```
+
+If there are no violations in a section, mark it ✓ in "Cleared". A FAIL verdict requires at least one CRITICAL. A NEEDS DISCUSSION verdict means no hard rule is broken but there is a pattern concern worth raising before merge.

.claude/agents/principal-engineer.md

@@ -0,0 +1,81 @@
+---
+name: principal-engineer
+description: Senior engineering design review. Use when you want judgment on whether an approach is architecturally sound, not just rule-compliant. Invoke before or after operator-reviewer when the change is non-trivial — new abstractions, new controllers, new cross-cutting patterns, significant refactors. Ask: "Use the principal-engineer agent to review this design."
+tools: Read, Grep, Glob
+model: claude-opus-4-7
+color: purple
+maxTurns: 25
+---
+
+You are a principal software engineer with deep experience building production Kubernetes operators. You review code and design decisions at a higher level than a rule-checklist: your job is to ask whether the approach is right, not just whether it follows the existing rules.
+
+You have read-only access to the codebase. Browse as much context as you need before forming an opinion. Do not rush to verdict.
+
+## What you evaluate
+
+### 1. Simplicity and necessity
+- Is this the simplest solution that correctly solves the problem?
+- Is any new abstraction, interface, or type actually necessary, or is it invented complexity?
+- Could this be done with less code and fewer moving parts?
+- If a helper was extracted, does it have a single clear reason to exist, or is it just factored-out noise?
+
+### 2. Abstraction fitness
+- Are the new types and interfaces at the right level? Do they express domain concepts (reconciliation, module state, SKR connectivity) or implementation details?
+- Does the naming reflect what the code *is* and *does*, without leaking the internal how?
+- Would a new contributor understand the intent from the type and method names alone, without reading the implementation?
+
+### 3. Architectural fit
+- Does this change follow the established patterns (interface injection, SSA, conditions, requeueing via `queue.DetermineRequeueInterval`)?
+- If it deviates from a pattern, is the deviation justified and localized, or does it set a precedent that will be copied incorrectly?
+- Does new state belong where it was placed? (Reconciler struct vs. service layer vs. caller)
+
+### 4. Error philosophy
+- Are errors wrapped with enough context to trace the failure without logs (`fmt.Errorf("context: %w", err)`)?
+- Is the error classification correct — transient vs. permanent, requeue vs. terminal?
+- Does the code distinguish between "caller did something wrong" and "external system is unavailable"?
+
+### 5. Observability
+- Are meaningful state transitions visible via conditions or metrics?
+- If this introduces new failure modes, is there a signal an oncall engineer can act on?
+- Is there a log statement at the right level (not too verbose, not silent on important transitions)?
+
+### 6. Concurrency and lifecycle
+- Is shared state protected? Are there hidden races in concurrent reconcile paths?
+- Does the change respect the SKR context lifecycle (Init → Get, InvalidateCache on auth failure)?
+- Are finalizers added before any work that creates external state that needs cleanup?
+
+### 7. Testability
+- Is the change testable with the existing test infrastructure (envtest, DualClusterFactory)?
+- Are new dependencies injectable as interfaces, or are they hardcoded concrete types?
+- Is the happy path tested? Is the primary failure mode tested?
+
+### 8. Maintenance cost
+- Will this code be easy to change in 12 months by someone who didn't write it?
+- Are there hidden assumptions that aren't expressed as types, constants, or comments?
+- Does this increase or decrease the cognitive load of the reconcile loop?
+
+## Output format
+
+```
+## Principal Engineer Review
+
+### Design assessment
+[2-4 sentences on the overall approach — is the design sound?]
+
+### Concerns
+- [HIGH] <file>:<line> — <design issue and why it matters>
+- [MEDIUM] <file>:<line> — <concern worth discussing>
+- [LOW] <file>:<line> — <minor observation>
+
+### What works well
+- <specific thing done right — be concrete, not just praise>
+
+### Verdict
+APPROVE / REQUEST CHANGES / REJECT
+
+[1-2 sentences on the decisive factor for the verdict]
+```
+
+A REJECT verdict means the fundamental approach needs rethinking before implementation details matter — suggest the alternative. REQUEST CHANGES means the approach is sound but specific design decisions need addressing. APPROVE means you would merge this, even if small things could be better.
+
+Do not give a verdict before you have read enough code to understand the context. If the diff alone is insufficient, read the surrounding files first.

.claude/cve-triage/context.md

@@ -0,0 +1,144 @@
+# CVE Triage Context — lifecycle-manager
+
+This file provides context for triaging CVEs raised by automated security scanners against lifecycle-manager. There are **two distinct CVE surfaces** — container/OS level and Go code level — each with different triage logic.
+
+---
+
+## Surface 1: Container images (BDBA findings)
+
+BDBA scans the built container image for OS-level and binary CVEs.
+
+### Images tracked
+
+#### lifecycle-manager (KCP operator)
+- **Registry**: `europe-docker.pkg.dev/kyma-project/prod/lifecycle-manager`
+- **Scanned tags**: `latest` and the two most recent release tags (see `sec-scanners-config.yaml`)
+- **Base image**: `gcr.io/distroless/static:nonroot` — no shell, no libc, no package manager, no APT
+- **Builder**: Go static binary (`CGO_ENABLED=0`, `GOFIPS140=v1.0.0`)
+- **Runtime user**: UID 65532 (nonroot)
+
+#### skr-webhook (runtime-watcher binary, deployed to SKR clusters)
+- **Source repo**: `github.com/kyma-project/runtime-watcher` — lifecycle-manager deploys it via `SKRWebhookManager`
+- **Role**: `ValidatingWebhook` on each SKR; forwards resource change events to KCP over mTLS
+- **TLS note**: `NextProtos: ["http/1.1"]` is a **CVE-2023-44487** (HTTP/2 Rapid Reset) mitigation — do not remove
+- For CVEs in the skr-webhook image, file issues against `kyma-project/runtime-watcher`
+
+### Image CVE triage logic
+
+1. **Is the vulnerable package present in the image?**
+   - `gcr.io/distroless/static` contains: `ca-certificates`, `tzdata`, `glibc` stubs only — no shell, no apt, no openssl, no libc runtime beyond what Go's static binary needs
+   - CVEs in bash, curl, wget, openssl, apt, python, etc. → **not applicable** (not in the image)
+   - When unsure: `docker run --rm --entrypoint=sh europe-docker.pkg.dev/.../lifecycle-manager:latest` will fail — there is no shell
+
+2. **Is it a glibc CVE?**
+   - `CGO_ENABLED=0` means the Go binary does not link against glibc — glibc CVEs → **not applicable**
+
+3. **No fix available for a present package?**
+   - Assess whether the vulnerable code path is reachable (network-accessible? requires auth? no shell = no local privilege escalation)
+   - Document assessment in the CVE tracking issue; suppression requires Kyma security team approval
+
+---
+
+## Surface 2: Go module dependencies (Mend SCA findings)
+
+Mend scans `go.mod` and the full module graph for known vulnerable versions. This is where most actionable findings come from.
+
+### Key Go module facts
+
+- Main module: `github.com/kyma-project/lifecycle-manager` (root `go.mod`)
+- API sub-module: `api/go.mod` — has its own dependency graph; Mend scans both
+- Test files excluded from Mend scan (`*_test.go`, `tests/`, `testutils/`)
+
+### Go module CVE triage logic
+
+1. **Is the module actually in the dependency graph?**
+   ```sh
+   go list -m -json all | jq 'select(.Path == "<module>")'
+   # Run from the affected module root (root or api/)
+   ```
+   If the module is not listed → **not applicable** (Mend may flag transitive deps from indirect paths not actually used).
+
+2. **Is it a direct or transitive dependency?**
+   ```sh
+   go mod why <module>
+   # Shows the import chain: lifecycle-manager → ... → vulnerable-module
+   ```
+   Transitive deps are harder to upgrade directly — check if the parent dependency has released a fix first.
+
+3. **Is the vulnerable code path reachable from production code?**
+   - Check `go mod why` import chain — if it only passes through test utilities → **not applicable in production**
+   - For network-parsing CVEs: check whether lifecycle-manager calls the vulnerable function with untrusted input
+
+4. **Is there a fixed version?**
+   - Check `go.mod` for a `replace` directive that may block the upgrade
+   - If fixed:
+     ```sh
+     GOFIPS140=v1.0.0 go get <module>@<fixed-version>
+     go mod tidy
+     make test  # verify nothing breaks
+     ```
+   - Open a `deps` PR: title format `deps: bump <module> to <version> (CVE-XXXX-XXXXX)`
+
+5. **FIPS constraint on crypto fixes** — critical:
+   - If the CVE is in a crypto package, the replacement **must be FIPS-140-approved**
+   - Do not substitute stdlib `crypto/*` with `golang.org/x/crypto` custom implementations — many are not FIPS-approved
+   - Safe: upgrading to a newer version of the same `crypto/tls`, `crypto/sha256`, etc. stdlib package
+   - Unsafe: replacing `crypto/rand` with a third-party PRNG, adding non-stdlib elliptic curve implementations
+   - When unsure: check the [Go FIPS module documentation](https://pkg.go.dev/crypto/internal/fips140) and consult the Kyma security team
+
+6. **No fix available?**
+   - Document: is it exploitable given lifecycle-manager's deployment context? (runs in KCP, not exposed to untrusted internet traffic directly)
+   - Add suppression to `sec-scanners-config.yaml` with justification — requires Kyma security team approval
+
+---
+
+## Surface 3: Go source code (Checkmarx One SAST findings)
+
+Checkmarx performs static analysis on Go source looking for code-level vulnerabilities: injection, path traversal, insecure deserialization, hardcoded secrets, etc.
+
+### Checkmarx scope
+
+- Preset: `go-default`
+- Excludes: `**/test/**`, `**/*_test.go`, `**/testutils/**`, `tests/**`
+- Scans: all production Go source in the main module
+
+### SAST CVE triage logic
+
+1. **Is the finding a true positive?**
+   - Checkmarx commonly false-positives on: log statements (not injection), label/annotation values (not executed), YAML marshalling (not SQL/shell injection)
+   - Verify: does the flagged code path accept input from an untrusted external source (HTTP request body, Kubernetes CR fields from untrusted users)?
+   - lifecycle-manager processes Kubernetes resources — its "input" comes from the Kubernetes API server, which enforces RBAC before any data reaches the operator
+
+2. **Common Go patterns that trigger false positives here:**
+   - `fmt.Sprintf` with CR field values → Checkmarx may flag as "string injection" — not exploitable if the result is used as a Kubernetes resource name (API server validates names)
+   - `os.ReadFile` with a config path set at startup → not a path traversal if the path is operator-controlled, not user-supplied
+
+3. **True positives to take seriously:**
+   - Hardcoded credentials or tokens in source (not test fixtures)
+   - `exec.Command` or `os.Exec` with any user-controlled input
+   - `crypto/md5` or `crypto/sha1` used for security purposes (not checksums)
+   - HTTP client that skips TLS verification (`InsecureSkipVerify: true`) outside of test code
+
+4. **Remediation pattern:**
+   - Fix in source, run `make lint` and `make test` before opening a PR
+   - If Checkmarx flags a pattern that is genuinely safe in context, add a suppression comment with justification — but only after confirming with the security team
+
+---
+
+## Scanner configuration reference
+
+Defined in `sec-scanners-config.yaml`:
+
+| Scanner | Type | Scope | Excludes |
+|---|---|---|---|
+| **BDBA** | Container binary scan | Production images | — |
+| **Mend** | Go module SCA | `go.mod` dependency graph | Test files |
+| **Checkmarx One** | Go SAST | Go source (`go-default` preset) | Test files |
+| **OSSF Scorecard** | Supply-chain security | GitHub repo posture | — (weekly) |
+
+## Key security constraints (do not weaken during fixes)
+
+- `GOFIPS140=v1.0.0` in `Dockerfile` and `Makefile` — never remove; removing it is not a valid CVE mitigation
+- TLS 1.3 minimum on all endpoints — do not downgrade even if a scanner recommends TLS 1.2 compatibility
+- `rotationPolicy: Always` on cert-manager certificates — required for key hygiene
+- Container base image pinned to sha256 digest — update the digest, never switch to a floating tag

.claude/rules/go-conventions.md

@@ -0,0 +1,26 @@
+---
+paths:
+  - "**/*.go"
+---
+
+# Go code conventions — lifecycle-manager
+
+`make lint` is the authoritative check. The full linter config is in `.golangci.yaml`.
+
+## Import aliases
+
+Strict aliases are enforced by `importas` — violations fail CI. The **complete alias list** is in `.golangci.yaml` under `linters-settings.importas.alias` (75 entries). When adding an import, check that file first.
+
+Import ordering is enforced by `gci`: **standard → third-party → project** (`github.com/kyma-project/lifecycle-manager`) **→ blank → dot**.
+
+## nolint policy
+
+Every `//nolint` directive **must** include an explanation:
+```go
+//nolint:funlen // composition root wiring — acceptable exception
+```
+Bare `//nolint:funlen` fails review. Check `.golangci.yaml` before adding any suppression.
+
+## FIPS
+
+Use `GOFIPS140=v1.0.0 go` for any `go` command run directly (the Makefile sets this automatically). Do not add dependencies that bypass the FIPS-approved stdlib crypto (no third-party elliptic curves, no custom cipher suites).

.github/workflows/check-generated-code.yml

@@ -0,0 +1,40 @@
+name: Check generated code
+
+permissions: { }
+
+on:
+  pull_request:
+    types: [ opened, synchronize, reopened, ready_for_review ]
+    paths:
+      - 'api/**'
+      - 'internal/**'
+      - 'config/crd/**'
+      - 'config/rbac/**'
+
+jobs:
+  check-generated:
+    name: "Verify generated files are up-to-date"
+    runs-on: ubuntu-latest
+    defaults:
+      run:
+        working-directory: lifecycle-manager
+    steps:
+      - name: Checkout lifecycle-manager
+        uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
+      - name: Set up Go
+        uses: actions/setup-go@7a3fe6cf4cb3a834922a1244abfce67bcef6a0c5 # v6.2.0
+        with:
+          go-version-file: 'lifecycle-manager/go.mod'
+      - name: Install controller-gen
+        run: make controller-gen
+      - name: Run make generate
+        run: make generate
+      - name: Run make manifests
+        run: make manifests
+      - name: Check for diff
+        run: |
+          if ! git diff --exit-code; then
+            echo ""
+            echo "Generated files are out of sync. Run 'make generate && make manifests' locally and commit the result."
+            exit 1
+          fi