Skip to content

secrets: support internal, name-keyed secrets alongside user secrets - #37996

Draft
jasonhernandez wants to merge 2 commits into
MaterializeInc:mainfrom
jasonhernandez:jason/ctp-secrets-internal
Draft

secrets: support internal, name-keyed secrets alongside user secrets#37996
jasonhernandez wants to merge 2 commits into
MaterializeInc:mainfrom
jasonhernandez:jason/ctp-secrets-internal

Conversation

@jasonhernandez

Copy link
Copy Markdown
Contributor

Motivation

PR 2 of 3 toward encrypting the cluster transport protocol (see #37995 for the stack overview). The controller needs to distribute system-generated credentials (a per-environment CA, per-replica TLS keys) to clusterd processes. The right channel already exists — environmentd writes secrets through SecretsController and every clusterd boots with a SecretsReader — but both traits are keyed by CatalogItemId, and transport credentials are not catalog items.

This PR adds internal secrets: name-keyed secrets for system-generated credentials, alongside the id-keyed user secrets. It is independent of #37995 and reviewable standalone; the wiring PR depends on both.

Design

  • ensure_internal(name, contents) / delete_internal(name) on SecretsController, read_internal(name) on SecretsReader.
  • Disjoint namespaces in every backend. Kubernetes: {prefix}internal-{name} next to the existing {prefix}user-managed-{id}. Process orchestrator: internal-{name} files next to bare {id} files. AWS: {prefix}internal-{name} next to {prefix}{id}. Internal secrets are excluded from list(), whose parsers only recognize user secret names.
  • Name validation (validate_internal_secret_name): lowercase alphanumerics and dashes only, so names are valid in all backends and cannot traverse paths. Enforced at every ensure_internal entry point.
  • The caching reader does not cache internal secrets. They are read rarely (typically once at process startup), so caching would only extend the lifetime of key material in memory.

Behavior changes

  • The process orchestrator's list() previously errored on any file in the secrets directory whose name did not parse as a CatalogItemId. It now skips such files, matching the documented behavior (and the Kubernetes backend's behavior) of ignoring unrecognized objects.
  • The AWS reader's log line now reports the full secret name rather than the bare id (the read path is shared between user and internal secrets).
  • ensure/delete/read bodies in the Kubernetes and AWS backends moved into name-keyed helpers shared with the internal variants; no functional change beyond the above.

Testing

  • mz-secrets: unit tests for the in-memory controller round-trip (including namespace disjointness with a user secret and idempotent deletion) and for name validation accept/reject cases.
  • mz-orchestrator-process: new integration test covering round-trip, overwrite, idempotent delete, list() skipping internal files instead of erroring, and rejection of path-traversal names.
  • The Kubernetes and AWS backends have no existing test harness; their internal-name mapping is pure string construction exercised end-to-end in the wiring PR.

🤖 Generated with Claude Code

jasonhernandez and others added 2 commits August 1, 2026 13:07
Extend SecretsController and SecretsReader with internal secrets,
keyed by name rather than by CatalogItemId. These hold
system-generated credentials, such as the upcoming CTP transport
keys, that have no corresponding catalog item.

The two namespaces are disjoint in every backend: Kubernetes uses an
"internal-" infix next to the existing "user-managed-" one, the
process orchestrator prefixes secret files with "internal-", and AWS
Secrets Manager gets an "internal-" infix after the deployment
prefix. Internal secrets are excluded from list(), whose parsers only
recognize user secret names. Names are validated against a
conservative alphabet (lowercase alphanumerics and dashes) that is
valid in all backends and cannot traverse paths.

The process orchestrator's list() previously errored on any file in
the secrets directory whose name did not parse as a CatalogItemId. It
now skips such files, matching the documented behavior of the other
backends.

The caching reader deliberately does not cache internal secrets. They
are read rarely, typically once at process startup, so caching would
only extend the lifetime of key material in memory.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Callers bootstrapping credentials must distinguish a secret that does
not exist yet (generate and persist a new one) from a transient read
failure (fail and retry). Conflating the two could cause a caller to
regenerate and overwrite live key material. Return Option instead of
an untyped error.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant