Skip to content

KServiceName() is non-injective: distinct (project, domain, name) can map to the same KService name (delimiter ambiguity + 32-bit suffix) #7622

Description

@3em0

Summary

KServiceName() derives the Knative Service name for an app from its (project, domain, name) identity, and all apps share the single flyte namespace. The current derivation is not injective — two different app identities can produce the same KService name. Combined with Deploy()'s Get → Create/Update (last-writer-wins) behavior, a second app that resolves to an already-used name will update the existing KService's spec rather than create a distinct object.

Filing this as a robustness/correctness concern. There's one thing I couldn't determine from the code (the access model) — see the question near the end; the answer decides whether this is only a same-tenant footgun or can cross ownership boundaries.

Observed in app/internal/k8s/app_client.go (commit 5df13b9).

The mapping

func KServiceName(id *flyteapp.Identifier) string {
    raw := strings.ToLower(fmt.Sprintf("%s-%s-%s", id.GetName(), id.GetProject(), id.GetDomain()))
    if len(raw) <= 63 { return raw }
    sum := sha256.Sum256([]byte(id.GetProject() + "/" + id.GetDomain() + "/" + id.GetName()))
    suffix := hex.EncodeToString(sum[:4])   // 4 bytes = 32-bit suffix
    prefix := raw
    if len(prefix) > 54 { prefix = prefix[:54] }
    return prefix + "-" + suffix
}

Observations

  1. Delimiter ambiguity (raw <= 63 path). The three fields are joined by -, and project/domain may themselves contain -, so the concatenation is ambiguous. Minimal illustration — both triples satisfy every declared field constraint, yet collide:

    • {name: "svc", project: "team", domain: "prod-x"}svc-team-prod-x
    • {name: "svc", project: "team-prod", domain: "x"}svc-team-prod-x
  2. 32-bit suffix (raw > 63 path). When raw exceeds the 63-char limit, uniqueness rests on a 4-byte (32-bit) SHA-256 suffix (sum[:4]). That's a small space to guarantee global uniqueness across all (project, domain, name) triples. The retained 54-char prefix does not add uniqueness against a specific existing name, because the leading bytes can be reproduced while only the tail (beyond position 54, e.g. in a long domain) is varied.

  3. Field-validation asymmetry. In flyteidl2/app/app_definition.proto, name is constrained (max_len = 30 + DNS-1123 label pattern), but project and domain carry only min_len = 1 — no max length and no charset restriction — so they can be arbitrarily long and contain -. I also didn't find a protovalidate interceptor wired on the app handler (only an otel interceptor in app/setup.go), so even these declarative constraints may not be enforced at the RPC boundary.

  4. Single shared namespace + last-writer-wins. All KServices live in namespace flyte, and Deploy() does Get(name); on hit it sets existing.Spec = ksvc.Spec, merges labels/annotations (including flyte.org/app-id), and Update()s. So a name collision means the later Create/Update overwrites the earlier app's KService spec instead of producing a separate object — an integrity concern for whoever created the name first.

Access-model question (couldn't determine from the code)

InternalAppService.Create validates only that id/spec/payload are non-nil; it doesn't check that the caller is entitled to the requested (project, domain). In the unified flyte binary (manager/cmd/main.go) the mux is wrapped only with corsMiddleware. Could a maintainer clarify the intended access model — is (project, domain) authorized against the caller's tenant somewhere upstream? That determines whether a name collision is confined to a single tenant or can affect another tenant's app.

Suggested hardening

  • Make the name derivation injective/unambiguous (length-prefix or escape the fields, or hash a canonical unambiguous encoding of the triple), and widen the truncated hash well beyond 32 bits.
  • Validate project/domain (max length + DNS-safe charset, mirroring name) and wire a protovalidate interceptor so the proto constraints are actually enforced.
  • Consider per-tenant namespaces rather than a single shared flyte namespace.
  • On Deploy, reject an update whose target KService is owned by a different app identity (compare the flyte.org/app-id annotation before Update).

Note on disclosure

Per SECURITY.md I'm glad to move this to the private security-advisory channel if you'd prefer to triage it there. I have a fuller local reproduction (a faithful copy of KServiceName() plus a Deploy() Get→Update simulation) that I can share privately.

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions