Skip to content

Audit: medium & low severity findings (13) #135

Description

@BryanFRD

This issue tracks the medium and low severity findings from the full-codebase audit of FerrVault. Critical and high findings are filed as individual issues. Each item below is independently fixable; check off as addressed.

Medium

  • [bug/operator] FerrVaultConnection Mode default disagrees between Go markers (ferrflow) and shipped CRD (ferrvault) — silent wrong-backend routing
    - C:/Users/bryan/Nextcloud/workspace/FerrFlow-Org/ferrflow-operator/api/v1alpha1/ferrflowconnection_types.go:29,113
    - The Go spec marker is +kubebuilder:default=ferrflow (api/v1alpha1/ferrflowconnection_types.go:29) and ResolvedMode() returns ferrflow for an empty Mode (lines 113-118). FerrVaultConnection embeds this exact spec (api/ferrvault/v1alpha1/ferrvaultconnection_types.go:22). But the hand-written CRD for ferrvaultconnections defaults mode: ferrvault (charts/.../crd-ferrvaultconnections.yaml:53-56…
    - Fix: Make the FerrVault path default to ferrvault explicitly (don't reuse FerrFlow's ResolvedMode default), and regenerate/align the CRD default with the Go marker so a regen can't silently flip backends. Add a test asserting an empty-mode FerrVaultConnection resolves to ferrvault.
  • [bug/operator] TokenBroker is never injected in main.go, so the OIDC cache is recreated per reconcile and the exchange runs on every reveal
    - C:/Users/bryan/Nextcloud/workspace/FerrFlow-Org/ferrflow-operator/cmd/main.go:89
    - All four reconcilers are constructed in cmd/main.go (lines 89-121) without setting Broker. Every code path that needs a token does broker := r.Broker; if broker == nil { broker = NewTokenBroker(r.Client) } (ferrflowsecret_controller.go:318-323, ferrvaultsecret_sync.go:20-30, ferrflowconnection_controller.go:190-193, ferrvaultconnection_controller.go:138-141). NewTokenBroker allocates a fresh e…
    - Fix: Construct one *TokenBroker (via NewTokenBroker(mgr.GetClient())) in main.go and pass the same pointer into all four reconcilers so the OIDC cache is shared and actually persists.
  • [bug/operator] Token read from a Secret is not whitespace-trimmed — trailing newline yields a malformed bearer and 401
    - C:/Users/bryan/Nextcloud/workspace/FerrFlow-Org/ferrflow-operator/internal/controller/token_broker.go:144
    - loadFromSecret returns string(raw) directly (token_broker.go:144-151) with no trimming, whereas the OIDC path does strings.TrimSpace(saToken) (token_broker.go:174). ferrflow.New only rejects an empty token, not whitespace (client.go:75-85). Secrets created the common way (kubectl create secret generic --from-file=token=token.txt, or many GitOps tools) carry a trailing newline, which then s…
    - Fix: strings.TrimSpace the value read from the token Secret (same as the OIDC path) before using it as the bearer.
  • [security/operator] Connection URL accepts plaintext http:// and arbitrary internal hosts — token sent in clear / SSRF to cluster-internal endpoints
    - C:/Users/bryan/Nextcloud/workspace/FerrFlow-Org/ferrflow-operator/internal/ferrflow/client.go:83
    - The connection URL is validated only against ^https?:// (api/v1alpha1/ferrflowconnection_types.go:37; CRD pattern in crd-ferrvaultconnections.yaml:62-64) and ferrflow.New accepts both http and https (client.go:83-85). The operator then sends Authorization: Bearer <SAT/cluster token> to that URL (client.go:255, 343). Any namespace user who can create a FerrVaultConnection can therefore (a) poin…
    - Fix: Require https:// for the connection URL (tighten the CRD pattern and the New() check), optionally with an explicit opt-out flag for air-gapped/dev like the CLI's gated insecure switch. Consider a denylist for link-local/metadata ranges.
  • [bug/operator] Two Secret-owning reconcilers run concurrently with no guard against managing the same target Secret
    - C:/Users/bryan/Nextcloud/workspace/FerrFlow-Org/ferrflow-operator/internal/controller/ferrvaultsecret_sync.go:32
    - FerrFlowSecretReconciler and FerrVaultSecretReconciler both Owns(&corev1.Secret{}) and both CreateOrUpdate a target Secret keyed by spec.target.name (defaulting to the CR name) in the same namespace (ferrflowsecret_controller.go:331-379 and ferrvaultsecret_sync.go:32-74). Nothing prevents a FerrFlowSecret and a FerrVaultSecret (or two of the same kind) from naming the same target Secret. Set…
    - Fix: Detect a pre-existing Secret not owned by this CR and refuse with a clear TargetConflict condition instead of fighting over ownership; optionally document that one target Secret maps to exactly one CR.

Low

  • [security/operator] Metrics endpoint served as unauthenticated plaintext HTTP on :8080
    - C:/Users/bryan/Nextcloud/workspace/FerrFlow-Org/ferrflow-operator/cmd/main.go:49
    - main.go binds the controller-runtime metrics server directly on :8080 (metrics-bind-address, main.go:49,78) with no TLS and no kube-rbac-proxy/authn-authz wrapper, and the deployment exposes it as a container port (charts/.../deployment.yaml:37,55-58). Exposed series include per-CR namespace/name labels (metrics.go:39-55), which leaks the inventory of which namespaces sync which secret CRs t…
    - Fix: Offer --metrics-secure / authn-authz on the metrics endpoint (controller-runtime's filtered metrics or kube-rbac-proxy sidecar), or document that it must be firewalled to Prometheus only.
  • [security/operator] SonarQube badge token committed in README
    - C:/Users/bryan/Nextcloud/workspace/FerrFlow-Org/ferrflow-operator/README.md:4
    - README.md badges embed token=sqb_d3cde9fc7a86f70e3652d22ce13b5a39522212c8 against sonar.ferrlabs.com (README.md:4-5). SonarQube project-badge tokens are low-sensitivity (read-only metric badges) but are still a credential checked into a public repo for a self-hosted Sonar instance; it should be rotated and badges switched to the tokenless form where possible.
    - Fix: Rotate the Sonar badge token and use unauthenticated badge URLs (or a CI-injected token), rather than committing it.
  • [ux/operator] FerrVaultConnection CRD requires organization but the field is documented as ignored in ferrvault mode
    - C:/Users/bryan/Nextcloud/workspace/FerrFlow-Org/ferrflow-operator/charts/ferrflow-operator/templates/crd-ferrvaultconnections.yaml:51
    - The FerrVaultConnection CRD lists organization in required: [url, organization] (crd-ferrvaultconnections.yaml:51) and the sample sets organization: my-org (config/samples/ferrvault_v1alpha1_ferrvaultconnection.yaml:8), yet the same CRD's own description (line 69) and the Go docstring (ferrflowconnection_types.go:24-26) state organization is ignored when mode=ferrvault because the SAT scopes…
    - Fix: Make organization optional for FerrVaultConnection (it's SAT-scoped), or validate/use it. Don't require a field you document as ignored.
  • [ux/operator] No XOR validation at admission for tokenSecretRef vs oidc — misconfig only surfaces at reconcile time
    - C:/Users/bryan/Nextcloud/workspace/FerrFlow-Org/ferrflow-operator/internal/controller/token_broker.go:114
    - TokenFor enforces 'exactly one of tokenSecretRef / oidc' at runtime (token_broker.go:114-132), but the CRDs make both optional with no oneOf/required constraint (crd-ferrvaultconnections.yaml:70-84, and the connection types doc says 'Exactly one authentication source must be set', ferrflowconnection_types.go:13-15). A Connection with neither, or both, is admitted and only fails later as a Toke…
    - Fix: Add a CEL validation rule (x-kubernetes-validations / oneOf) on the connection CRD so applying a spec with zero or both auth sources is rejected at admission.
  • [security/operator] API error bodies are echoed verbatim into world-readable CR status messages
    - C:/Users/bryan/Nextcloud/workspace/FerrFlow-Org/ferrflow-operator/internal/ferrflow/client.go:502
    - On non-2xx responses the client extracts the server's {"error":...} string (client.go:502-512) and the reconcilers stamp it straight into the Ready condition Message via failReady*(...err.Error()) (ferrvaultsecret_controller.go:121-126, ferrflowsecret_controller.go:191-202). Anyone with get ferrvaultsecret/describe in the namespace sees whatever the FerrVault API returned. This is fine only …
    - Fix: Map upstream errors to a fixed set of safe reasons/messages for the CR status and log the raw body at debug only, rather than reflecting arbitrary server text into a namespace-readable status.
  • [docs/operator] Module path, folder, RBAC group names, and README still say ferrflow.io despite the FerrVault rename
    - C:/Users/bryan/Nextcloud/workspace/FerrFlow-Org/ferrflow-operator/README.md:9
    - CLAUDE.md states this repo is FerrLabs/FerrVault and the operator is the FerrVault operator (CRDs renamed from FerrFlow*, breaking change pending). Yet the Go module is github.com/FerrLabs/FerrFlow-Operator (go.mod:1), the chart/RBAC names and leader-election lease are ferrflow-operator / ferrflow-operator.ferrflow.io (clusterrole.yaml:5, values.yaml:22, main.go:56), the README top still des…
    - Fix: Finish the rename pass: update README to document the FerrVault CRDs that ship, reconcile the SAT prefix (sat_ vs fvsat_) between code, samples and threat model, and track the module/chart/RBAC renames as the pending breaking change.
  • [docs/operator] Threat model documents only the Rust CLI; the operator's reveal/auth surface is undocumented
    - C:/Users/bryan/Nextcloud/workspace/FerrFlow-Org/ferrflow-operator/docs/SECURITY-THREAT-MODEL.md:1
    - docs/SECURITY-THREAT-MODEL.md scopes itself to 'the ferrvault consumer CLI shipped from this repository, plus the GitHub Action wrapper' (lines 1-5) and the entire asset/threat table is CLI-centric (keyring, child-process env, etc.). The operator — which holds long-lived SATs/cluster tokens in K8s Secrets, performs OIDC exchange, egresses bearers to a user-supplied URL, and writes decrypted values…
    - Fix: Add an operator threat-model section (token at rest in Secrets, OIDC exchange trust, egress URL validation, target-Secret RBAC, metrics exposure) or split into a dedicated operator doc.
  • [missing-feature/operator] No secret rotation trigger / drift correction is purely time-based with a 1h default
    - C:/Users/bryan/Nextcloud/workspace/FerrFlow-Org/ferrflow-operator/internal/controller/ferrflowsecret_controller.go:463
    - Reconciliation against upstream only happens on spec change, on token-Secret/Connection watch events, or on the refreshInterval timer (default 1h, ferrflowsecret_controller.go:287,463-472; values.yaml:31). There's no on-demand 'sync now' mechanism (e.g. an annotation bump the operator watches, or an API-push webhook) when a secret rotates upstream in FerrVault — a rotated credential can take up …
    - Fix: Support a manual refresh trigger (e.g. reconcile on a ferrvault.com/force-sync annotation change) and/or an upstream push/webhook so rotations propagate without waiting a full interval.

Found during a full-codebase audit of the FerrLabs workspace.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions