Skip to content

CryptoContext.namespace / .key document themselves as settings-only, one layer below sys_secret's three producers #12599

Description

@os-warren

Observation-class finding, surfaced while fixing #12550 (sys_secret field help). ⛔ Not assigned, not queued — recording drift.

What is declared vs what is true

#12550 corrects the shipped field descriptions on sys_secret, whose (namespace, key) pair means something different per producer. The contract those three producers all call carries the same settings-only prose one layer down, in packages/spec/src/contracts/crypto-provider.ts:

export interface CryptoHandle {
  /** Stable opaque id stored in `sys_setting.value_enc`. */
  readonly id: string;
  ...
}

export interface CryptoContext {
  /** Settings namespace the value belongs to. */
  namespace: string;
  /** Specifier key within the namespace. */
  key: string;
  ...
}

export interface ICryptoProvider {
  /**
   * Encrypt plaintext and return a handle. The handle is stored in
   * `sys_secret` and referenced by `sys_setting.value_enc`.
   */
  encrypt(plain: string, ctx: CryptoContext): Promise<CryptoHandle>;

Measured on origin/main@f93df4db, by call site rather than by line number — all three producers construct a CryptoContext, and only one of them means "settings":

caller ctx.namespace ctx.key where the handle id ends up
SettingsService (services/service-settings/src/settings-service.ts) settings namespace specifier key sys_setting.value_enc
engine encryptSecretFields (packages/objectql/src/engine.ts) object name field name a secret:<id> ref on the business row itself
datasource credential binder (services/service-datasource/src/datasource-secret-binder.ts) caller-supplied, default datasource datasource name the artefact's sys_secret:<id> credentialsRef

So CryptoHandle.id is not "stored in sys_setting.value_enc" in general, and CryptoContext.namespace is not a settings namespace in general.

Why it is worth writing down

Unlike #12550's strings these are TS doc comments, not published metadata — nothing ships them to an operator, which is why this is filed separately and graded as an observation rather than folded into that PR (different package, different regeneration/gate family).

What makes it more than a typo is the sentence just above CryptoContext: "so providers can implement Additional Authenticated Data (AAD) bindings — e.g. AWS KMS encryption context. Helps reject ciphertexts that were copied across namespaces." A provider author implementing that binding is being told the pair is a settings coordinate, while in fact one flat (namespace, key) space is shared by three vocabularies — an object named datasource, or a settings namespace colliding with an object name, is not excluded by anything here. Whether that is merely a doc fix or has a real AAD-collision consequence is the question a triager should decide; this card only records that the prose asserts the single-producer reading.

Shape of a fix

Doc-comment corrections in packages/spec/src/contracts/crypto-provider.ts naming the producer-scoped reality (the same correction #12550 made to the shipped descriptions), plus whatever the AAD question turns out to need. packages/spec edits carry their own artifact-regeneration discipline, so it is not a rider on #12550's PR.

Found while working #12550; filed unassigned for triage.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions