From 9a57a1ed51c24aa9d29441fb91571c63caaef9c3 Mon Sep 17 00:00:00 2001 From: Maxime Beauchamp <15185355+baktun14@users.noreply.github.com> Date: Sat, 17 Oct 2026 19:00:54 -0700 Subject: [PATCH 1/2] feat(deployment): open hidden variables sealed under a team's data key A stored token whose header names an organization instead of a member opens under the data key and binding of the organization its row is filed in. Patching, redeploying, inheriting and sending the manifest of a team deployment read it that way, including after the member who filed the row deleted their account. A user re-key leaves such a row to the team and counts it apart. Nothing seals under a team's key yet, so every stored token still opens as before. Part of CON-1213 --- apps/api/scripts/rehearseDeploymentSecrets.ts | 4 +- .../deployment-setting.repository.ts | 10 +- .../deployment-writer.service.spec.ts | 22 +++- .../deployment-writer.service.ts | 34 ++++-- .../lease-manifest.service.spec.ts | 7 +- .../lease-manifest/lease-manifest.service.ts | 11 +- .../sdl-secrets-inheritance.service.spec.ts | 37 +++++- .../sdl-secrets-inheritance.service.ts | 32 +++-- .../sdl-secrets/sdl-secrets.service.spec.ts | 111 +++++++++++++++--- .../sdl-secrets/sdl-secrets.service.ts | 59 ++++++++-- .../data-key-rekey.service.integration.ts | 5 +- .../data-key-rekey.service.spec.ts | 34 +++++- .../data-key-rekey/data-key-rekey.service.ts | 14 ++- .../functional/deployment-definition.spec.ts | 4 +- .../test/functional/deployment-patch.spec.ts | 4 +- .../deployment-sealed-secrets.spec.ts | 7 +- apps/api/test/functional/deployments.spec.ts | 2 +- .../functional/organization-wallet.spec.ts | 2 +- 18 files changed, 318 insertions(+), 81 deletions(-) diff --git a/apps/api/scripts/rehearseDeploymentSecrets.ts b/apps/api/scripts/rehearseDeploymentSecrets.ts index 9f76bf5345..0ce523b585 100644 --- a/apps/api/scripts/rehearseDeploymentSecrets.ts +++ b/apps/api/scripts/rehearseDeploymentSecrets.ts @@ -251,7 +251,9 @@ async function inspectDeployment(userId: string, row: DeploymentStoredSecretsOfU const kid = kidOf(row.sealedSecrets); try { - const secrets = await container.resolve(SdlSecretsService).openStored({ userId, dseq: row.dseq, sealedSecrets: row.sealedSecrets }); + const secrets = await container + .resolve(SdlSecretsService) + .openStored({ userId, organizationId: row.organizationId, dseq: row.dseq, sealedSecrets: row.sealedSecrets }); return { dseq: row.dseq, kid, opens: true, secretCount: Object.keys(secrets).length }; } catch (error) { diff --git a/apps/api/src/deployment/repositories/deployment-setting/deployment-setting.repository.ts b/apps/api/src/deployment/repositories/deployment-setting/deployment-setting.repository.ts index f272acfed2..7bb154db26 100644 --- a/apps/api/src/deployment/repositories/deployment-setting/deployment-setting.repository.ts +++ b/apps/api/src/deployment/repositories/deployment-setting/deployment-setting.repository.ts @@ -145,7 +145,7 @@ export type DeploymentStoredSecrets = { }; /** A user's stored secrets token with the deployment it is bound to, which re-sealing it under another key has to name. */ -export type DeploymentStoredSecretsOfUser = DeploymentStoredSecrets & { dseq: string }; +export type DeploymentStoredSecretsOfUser = DeploymentStoredSecrets & Pick; export type StoredLeaseGpus = { readings: LeaseGpuReading[]; offers: LeaseGpuOffer[] }; @@ -840,7 +840,13 @@ export class DeploymentSettingRepository extends OrgScopedRepository { await service.patchByUserIdAndDseq("user-1", "1234", { services: { web: { image: "nginx:1.27" } } }, ability); - expect(sdlSecretsService.openStored).toHaveBeenCalledWith({ userId: "user-1", dseq: "1234", sealedSecrets: STORED_TOKEN }); + expect(sdlSecretsService.openStored).toHaveBeenCalledWith({ + userId: "user-1", + organizationId: "personal-organization-1", + dseq: "1234", + sealedSecrets: STORED_TOKEN + }); }); it("opens the client's seal against the sdl the console stored, which is what it was sealed to", async () => { @@ -4404,7 +4409,12 @@ describe(DeploymentWriterService.name, () => { await service.applySavedPatch(savedPatch, ability); - expect(sdlSecretsService.openStored).toHaveBeenCalledWith({ userId: "user-1", dseq: "100", sealedSecrets: "stored.token.aaa.bbb.ccc" }); + expect(sdlSecretsService.openStored).toHaveBeenCalledWith({ + userId: "user-1", + organizationId: "organization-of-the-row", + dseq: "100", + sealedSecrets: "stored.token.aaa.bbb.ccc" + }); }); it("rebuilds the manifest from the saved sdl and the values its token holds", async () => { @@ -4603,7 +4613,12 @@ describe(DeploymentWriterService.name, () => { expect(signerService.filedUserIdOf).toHaveBeenCalledWith(organizationWallet, "user-1", "100"); expect(scopedSettingRepository.findOneBy).toHaveBeenCalledWith({ userId: "filer-1", dseq: "100" }); - expect(sdlSecretsService.openStored).toHaveBeenCalledWith({ userId: "filer-1", dseq: "100", sealedSecrets: "stored.token.aaa.bbb.ccc" }); + expect(sdlSecretsService.openStored).toHaveBeenCalledWith({ + userId: "filer-1", + organizationId: "organization-of-the-row", + dseq: "100", + sealedSecrets: "stored.token.aaa.bbb.ccc" + }); }); it("updates the chain and pushes the manifest with the organization's wallet, for the member who saved the patch", async () => { @@ -4951,6 +4966,7 @@ describe(DeploymentWriterService.name, () => { sdl: input?.storedSdl ?? SDL_OF_A_REDEPLOY, runtimeLimitHours: input?.storedRuntimeLimitHours ?? null, sealedSecrets: "stored.token.aaa.bbb.ccc", + organizationId: "organization-of-the-row", manifestVersion: input?.storedManifestVersion ?? savedVersion }) ); diff --git a/apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts b/apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts index 5871643f1d..a26773c1e5 100644 --- a/apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts +++ b/apps/api/src/deployment/services/deployment-writer/deployment-writer.service.ts @@ -200,6 +200,15 @@ type PreparedPatch = { assertMayStillSign?: () => Promise; }; +type StoredDefinition = { + sdl: string; + sealedSecrets: string | null; + manifestVersion?: string; + runtimeLimitHours?: number; + /** Absent for a deployment filed before organizations, whose row only its user reaches. */ + organizationId?: string; +}; + type PatchWrite = { definition: Parameters[0]; counts: { patchedServiceCount: number; secretCount: number; guarded: boolean }; @@ -981,7 +990,9 @@ export class DeploymentWriterService { const [supplied, held] = await Promise.all([ input.sealedSecrets ? this.sdlSecretsService.receiveForMerge({ rawSdl: stored.sdl, sealedSecrets: input.sealedSecrets }) : {}, - stored.sealedSecrets ? this.sdlSecretsService.openStored({ userId: filedUserId, dseq, sealedSecrets: stored.sealedSecrets }) : {} + stored.sealedSecrets + ? this.sdlSecretsService.openStored({ userId: filedUserId, organizationId: stored.organizationId ?? null, dseq, sealedSecrets: stored.sealedSecrets }) + : {} ]); const merged = this.#mergeAndPrune({ held, supplied, derived }, document); @@ -1027,11 +1038,7 @@ export class DeploymentWriterService { } /** Read from the row even right after the save, so a patch a newer one replaced is never applied on chain, however late it runs. */ - async #findDefinitionSavedAs( - saved: SavedPatch, - filedUserId: string, - ability: AnyAbility - ): Promise<{ sdl: string; sealedSecrets: string | null } | undefined> { + async #findDefinitionSavedAs(saved: SavedPatch, filedUserId: string, ability: AnyAbility): Promise { const definition = await this.#findStoredDefinition({ userId: filedUserId, dseq: saved.dseq }, ability, NOT_PATCHABLE_MESSAGE); if (definition.manifestVersion === saved.manifestVersion) return definition; @@ -1044,10 +1051,15 @@ export class DeploymentWriterService { /** Resolved without trial limits, which refuse a patch when it is saved and never change the manifest it hashes. */ async #prepareSavedDefinition( { saved, wallet, filedUserId }: { saved: SavedPatch; wallet: WalletInitialized; filedUserId: string }, - definition: { sdl: string; sealedSecrets: string | null } + definition: StoredDefinition ): Promise { const secrets = definition.sealedSecrets - ? await this.sdlSecretsService.openStored({ userId: filedUserId, dseq: saved.dseq, sealedSecrets: definition.sealedSecrets }) + ? await this.sdlSecretsService.openStored({ + userId: filedUserId, + organizationId: definition.organizationId ?? null, + dseq: saved.dseq, + sealedSecrets: definition.sealedSecrets + }) : {}; const resolved = await this.#resolveSdl(definition.sdl, { secrets }); const deployment = await this.deploymentReaderService.findByWalletAndDseqWithoutProviderStatus(wallet, saved.dseq); @@ -1173,11 +1185,7 @@ export class DeploymentWriterService { return { ...deployment, name: persistedName }; } - async #findStoredDefinition( - key: { userId: string; dseq: string }, - ability: AnyAbility, - notFoundMessage: string - ): Promise<{ sdl: string; sealedSecrets: string | null; manifestVersion?: string; runtimeLimitHours?: number; organizationId?: string }> { + async #findStoredDefinition(key: { userId: string; dseq: string }, ability: AnyAbility, notFoundMessage: string): Promise { const setting = await this.deploymentSettingRepository.accessibleBy(ability, "read").findOneBy(key); if (!setting?.sdl) { diff --git a/apps/api/src/deployment/services/lease-manifest/lease-manifest.service.spec.ts b/apps/api/src/deployment/services/lease-manifest/lease-manifest.service.spec.ts index 6d82ab8e1f..6a0f2be0ec 100644 --- a/apps/api/src/deployment/services/lease-manifest/lease-manifest.service.spec.ts +++ b/apps/api/src/deployment/services/lease-manifest/lease-manifest.service.spec.ts @@ -23,6 +23,7 @@ import { mockConfigService } from "@test/mocks/config-service.mock"; const USER_ID = "3f1b6d4e-0000-4000-8000-000000000001"; const DSEQ = "1420000000001"; +const ORGANIZATION_ID = "5c2a7e1f-0000-4000-8000-000000000002"; function sdlWith(env: string[], options: { credentials?: Record; gpuModel?: string } = {}) { const credentialsBlock = options.credentials @@ -184,7 +185,7 @@ describe(LeaseManifestService.name, () => { await service.deriveFor({ dseq: DSEQ, userId: USER_ID }); - expect(sdlSecretsService.openStored).toHaveBeenCalledWith({ userId: USER_ID, dseq: DSEQ, sealedSecrets: "sealed" }); + expect(sdlSecretsService.openStored).toHaveBeenCalledWith({ userId: USER_ID, organizationId: ORGANIZATION_ID, dseq: DSEQ, sealedSecrets: "sealed" }); }); it("logs what it derived and no part of what it resolved", async () => { @@ -400,7 +401,9 @@ describe(LeaseManifestService.name, () => { deploymentSettingRepository.accessibleBy.mockReturnValue(scopedRepository); scopedRepository.findOneBy.mockResolvedValue( - input.definition ? mock({ sdl: input.definition.sdl, sealedSecrets: input.definition.sealedSecrets }) : undefined + input.definition + ? mock({ sdl: input.definition.sdl, sealedSecrets: input.definition.sealedSecrets, organizationId: ORGANIZATION_ID }) + : undefined ); const createLogger: CreateLogger = () => logger; diff --git a/apps/api/src/deployment/services/lease-manifest/lease-manifest.service.ts b/apps/api/src/deployment/services/lease-manifest/lease-manifest.service.ts index 790ebc5a17..8af83d256d 100644 --- a/apps/api/src/deployment/services/lease-manifest/lease-manifest.service.ts +++ b/apps/api/src/deployment/services/lease-manifest/lease-manifest.service.ts @@ -10,7 +10,7 @@ import { SdlService } from "@src/deployment/services/sdl/sdl.service"; import { SdlReferenceService } from "@src/deployment/services/sdl-reference/sdl-reference.service"; import { SdlSecretsService } from "@src/deployment/services/sdl-secrets/sdl-secrets.service"; -type StoredDefinition = { sdl: string; sealedSecrets: string | null }; +type StoredDefinition = { sdl: string; sealedSecrets: string | null; organizationId: string | null }; /** Why a deployment has no manifest of its own, which is what the residual volume of client-supplied ones is measured by. */ type StoredDefinitionMiss = "nothing-recorded" | "unresolvable"; @@ -48,7 +48,8 @@ export class LeaseManifestService { return this.#fallBack({ userId, dseq, reason: "nothing-recorded" }); } - const secrets = definition.sealedSecrets ? await this.#openStored(definition.sealedSecrets, { userId, dseq }) : {}; + const { sealedSecrets, organizationId } = definition; + const secrets = sealedSecrets ? await this.#openStored({ sealedSecrets, organizationId }, { userId, dseq }) : {}; const resolved = await this.sdlService.generateResolvedManifest({ sdl: definition.sdl, secrets }); @@ -65,7 +66,7 @@ export class LeaseManifestService { async #findDefinition(key: { userId: string; dseq: string }): Promise { const setting = await this.deploymentSettingRepository.accessibleBy(this.authService.ability, "read").findOneBy(key); - if (setting?.sdl) return { sdl: setting.sdl, sealedSecrets: setting.sealedSecrets }; + if (setting?.sdl) return { sdl: setting.sdl, sealedSecrets: setting.sealedSecrets, organizationId: setting.organizationId }; if (setting?.sealedSecrets) throw this.#refuse(key); @@ -95,9 +96,9 @@ export class LeaseManifestService { } /** Keeps the cause's own error, so a key service that is merely unreachable still answers 503 rather than reading as a definition that cannot be derived. */ - async #openStored(sealedSecrets: string, key: { userId: string; dseq: string }) { + async #openStored(stored: { sealedSecrets: string; organizationId: string | null }, key: { userId: string; dseq: string }) { try { - return await this.sdlSecretsService.openStored({ ...key, sealedSecrets }); + return await this.sdlSecretsService.openStored({ ...key, ...stored }); } catch (error) { throw isRetryable(error) ? this.#reportUnreachable(key, error) : this.#refuse(key, error); } diff --git a/apps/api/src/deployment/services/sdl-secrets-inheritance/sdl-secrets-inheritance.service.spec.ts b/apps/api/src/deployment/services/sdl-secrets-inheritance/sdl-secrets-inheritance.service.spec.ts index 99d26171ef..891b3a1549 100644 --- a/apps/api/src/deployment/services/sdl-secrets-inheritance/sdl-secrets-inheritance.service.spec.ts +++ b/apps/api/src/deployment/services/sdl-secrets-inheritance/sdl-secrets-inheritance.service.spec.ts @@ -30,7 +30,7 @@ describe(SdlSecretsInheritanceService.name, () => { await service.open(REQUEST); - expect(sdlSecretsService.openStored).toHaveBeenCalledWith({ ...SOURCE, sealedSecrets: TOKEN }); + expect(sdlSecretsService.openStored).toHaveBeenCalledWith({ ...SOURCE, organizationId: null, sealedSecrets: TOKEN }); }); it("finds the source through the wallet that owns it, only where the caller may change it", async () => { @@ -43,11 +43,13 @@ describe(SdlSecretsInheritanceService.name, () => { }); it("opens an organization's source under the member it was filed by", async () => { - const { service, sdlSecretsService } = setup({ setting: mock({ sealedSecrets: TOKEN, userId: "filer-1" }) }); + const { service, sdlSecretsService } = setup({ + setting: mock({ sealedSecrets: TOKEN, userId: "filer-1", organizationId: "organization-1" }) + }); await service.open({ wallet: { userId: null, organizationId: "organization-1" }, dseq: SOURCE.dseq }); - expect(sdlSecretsService.openStored).toHaveBeenCalledWith({ userId: "filer-1", dseq: SOURCE.dseq, sealedSecrets: TOKEN }); + expect(sdlSecretsService.openStored).toHaveBeenCalledWith({ userId: "filer-1", organizationId: "organization-1", dseq: SOURCE.dseq, sealedSecrets: TOKEN }); }); it("inherits from a closed deployment, because closing a deployment keeps its secrets", async () => { @@ -92,6 +94,26 @@ describe(SdlSecretsInheritanceService.name, () => { expect(sdlSecretsService.openStored).not.toHaveBeenCalled(); }); + it("answers not found for a team's source whose filer has deleted their account and that holds no token", async () => { + const { service } = setup({ setting: mock({ sealedSecrets: null, userId: null }) }); + + await expect(service.open({ wallet: { userId: null, organizationId: "organization-1" }, dseq: SOURCE.dseq })).rejects.toMatchObject({ status: 404 }); + }); + }); + + describe("a team's source whose token is sealed under the team's key", () => { + it("opens it after its filer has deleted their account, since the team's key outlives them", async () => { + const carried = { TOKEN: faker.string.alphanumeric(32) }; + const sealedSecrets = tokenSealedForTeam("organization-1"); + const { service, sdlSecretsService } = setup({ + setting: mock({ sealedSecrets, userId: null, organizationId: "organization-1" }), + opened: carried + }); + + await expect(service.open({ wallet: { userId: null, organizationId: "organization-1" }, dseq: SOURCE.dseq })).resolves.toEqual(carried); + expect(sdlSecretsService.openStored).toHaveBeenCalledWith({ userId: null, organizationId: "organization-1", dseq: SOURCE.dseq, sealedSecrets }); + }); + it("says nothing about whether the deployment exists", async () => { const { service } = setup({ setting: undefined }); @@ -241,12 +263,19 @@ describe(SdlSecretsInheritanceService.name, () => { expect(JSON.stringify(vi.mocked(logger.info).mock.calls)).not.toContain(value); }); + function tokenSealedForTeam(organizationId: string) { + const header = Buffer.from(JSON.stringify({ org: organizationId, dseq: SOURCE.dseq, alg: "dir", enc: "A256GCM", kid: "team-key" })).toString("base64url"); + + return `${header}..iv.ciphertext.tag`; + } + function unreadableTokenError() { return createError(500, SECRET_UNREADABLE_ERROR_MESSAGE, { errorCode: SECRET_UNREADABLE_ERROR_CODE }); } function setup(input?: { setting?: DeploymentSettingsOutput; opened?: SdlSecrets; openStoredError?: Error }) { - const setting = "setting" in (input ?? {}) ? input!.setting : mock({ sealedSecrets: TOKEN, userId: SOURCE.userId }); + const setting = + "setting" in (input ?? {}) ? input!.setting : mock({ sealedSecrets: TOKEN, userId: SOURCE.userId, organizationId: null }); const scoped = mock(); scoped.findOneOfWallet.mockResolvedValue(setting); diff --git a/apps/api/src/deployment/services/sdl-secrets-inheritance/sdl-secrets-inheritance.service.ts b/apps/api/src/deployment/services/sdl-secrets-inheritance/sdl-secrets-inheritance.service.ts index d6a4803648..7f8a8eb645 100644 --- a/apps/api/src/deployment/services/sdl-secrets-inheritance/sdl-secrets-inheritance.service.ts +++ b/apps/api/src/deployment/services/sdl-secrets-inheritance/sdl-secrets-inheritance.service.ts @@ -3,8 +3,17 @@ import { inject, singleton } from "tsyringe"; import { AuthService } from "@src/auth/services/auth.service"; import { type CreateLogger, LOGGER_FACTORY } from "@src/core"; -import { DeploymentSettingRepository, type DeploymentWallet } from "@src/deployment/repositories/deployment-setting/deployment-setting.repository"; -import { SdlSecretsService, STORED_SECRETS_UNAVAILABLE_ERROR_CODE } from "@src/deployment/services/sdl-secrets/sdl-secrets.service"; +import { + DeploymentSettingRepository, + type DeploymentSettingsOutput, + type DeploymentWallet +} from "@src/deployment/repositories/deployment-setting/deployment-setting.repository"; +import { + isSealedForOrganization, + SdlSecretsService, + STORED_SECRETS_UNAVAILABLE_ERROR_CODE, + type StoredSecretsOfDeployment +} from "@src/deployment/services/sdl-secrets/sdl-secrets.service"; import type { SdlSecrets } from "@src/deployment/services/sdl-secrets-unsealer/sdl-secrets-unsealer.service"; import { SECRET_UNREADABLE_ERROR_CODE } from "@src/secret/config/secret-at-rest.config"; @@ -31,6 +40,11 @@ function isTokenUnreadable(error: unknown): boolean { return isHttpError(error) && (error.errorCode === SECRET_UNREADABLE_ERROR_CODE || isSealedForAnotherMember(error)); } +/** A filer who deleted their account took their data key along, so without them only a token sealed under the team's key still opens. */ +function outlivesItsFiler(setting: Pick): boolean { + return setting.userId !== null || (setting.sealedSecrets !== null && isSealedForOrganization(setting.sealedSecrets)); +} + /** * Opens the token another deployment of the same owner holds, so a redeploy can carry its values forward. * Reads nothing but the token: the source's SDL is never consulted, and its `closed` state is deliberately @@ -50,11 +64,11 @@ export class SdlSecretsInheritanceService { this.#loggerService = createLogger({ context: SdlSecretsInheritanceService.name }); } - /** The source is found through the wallet that owns it and must be writable under the caller's ability, so one the caller may not change, or whose filer's account is deleted, is unreachable rather than merely refused. */ + /** The source is found through the wallet that owns it and must be writable under the caller's ability, so one the caller may not change, or whose secrets left with its filer's deleted account, is unreachable rather than merely refused. */ async open({ wallet, dseq }: { wallet: DeploymentWallet; dseq: string }): Promise { const setting = await this.deploymentSettingRepository.accessibleBy(this.authService.ability, "update").findOneOfWallet(wallet, dseq); - if (!setting?.userId) { + if (!setting || !outlivesItsFiler(setting)) { this.#loggerService.warn({ event: "SDL_SECRETS_INHERIT_SOURCE_NOT_FOUND", userId: wallet.userId, organizationId: wallet.organizationId, dseq }); throw createError(404, SOURCE_NOT_FOUND_MESSAGE); @@ -68,7 +82,7 @@ export class SdlSecretsInheritanceService { return {}; } - const inherited = await this.#openStored(setting.sealedSecrets, key); + const inherited = await this.#openStored({ ...key, organizationId: setting.organizationId, sealedSecrets: setting.sealedSecrets }); this.#loggerService.info({ event: "SDL_SECRETS_INHERITED", ...key, inheritedCount: Object.keys(inherited).length }); @@ -76,18 +90,18 @@ export class SdlSecretsInheritanceService { } /** Only the cipher's own permanent answer is rewritten: a 503 stays retryable, and a failure from anywhere else must not masquerade as secrets that are gone. */ - async #openStored(sealedSecrets: string, key: { userId: string; dseq: string }): Promise { + async #openStored(stored: StoredSecretsOfDeployment): Promise { try { - return await this.sdlSecretsService.openStored({ ...key, sealedSecrets }); + return await this.sdlSecretsService.openStored(stored); } catch (error) { if (!isPermanentlyUnreadable(error)) throw error; - throw this.#rejectUnreadable(error, key); + throw this.#rejectUnreadable(error, { userId: stored.userId, dseq: stored.dseq }); } } /** Carries the cause into both the log and the error, because the message a caller receives says only that the secrets will not open and never why. */ - #rejectUnreadable(error: unknown, key: { userId: string; dseq: string }) { + #rejectUnreadable(error: unknown, key: { userId: string | null; dseq: string }) { const event = isTokenUnreadable(error) ? "SDL_SECRETS_INHERIT_TOKEN_UNREADABLE" : "SDL_SECRETS_INHERIT_STATE_UNREADABLE"; this.#loggerService.error({ event, ...key, error }); diff --git a/apps/api/src/deployment/services/sdl-secrets/sdl-secrets.service.spec.ts b/apps/api/src/deployment/services/sdl-secrets/sdl-secrets.service.spec.ts index b4357926d7..f14321a8ec 100644 --- a/apps/api/src/deployment/services/sdl-secrets/sdl-secrets.service.spec.ts +++ b/apps/api/src/deployment/services/sdl-secrets/sdl-secrets.service.spec.ts @@ -9,8 +9,9 @@ import type { DeploymentConfigService } from "@src/deployment/services/deploymen import { MAX_SDL_REFERENCE_NAME_LENGTH, SdlReferenceService } from "@src/deployment/services/sdl-reference/sdl-reference.service"; import type { SdlSecrets } from "@src/deployment/services/sdl-secrets-unsealer/sdl-secrets-unsealer.service"; import type { SdlSecretsUnsealerService } from "@src/deployment/services/sdl-secrets-unsealer/sdl-secrets-unsealer.service"; +import { SECRET_UNREADABLE_ERROR_CODE } from "@src/secret/config/secret-at-rest.config"; import type { SecretCipherService } from "@src/secret/services/secret-cipher/secret-cipher.service"; -import { SdlSecretsService, STORED_SECRETS_UNAVAILABLE_ERROR_CODE } from "./sdl-secrets.service"; +import { isSealedForOrganization, SdlSecretsService, STORED_SECRETS_UNAVAILABLE_ERROR_CODE } from "./sdl-secrets.service"; import { mockConfigService } from "@test/mocks/config-service.mock"; @@ -491,7 +492,7 @@ describe(SdlSecretsService.name, () => { it("opens the token under the binding the seal was written with", async () => { const { service, secretCipherService } = setup({ stored: { TOKEN: "one" } }); - await service.openStored({ userId: "user-1", dseq: "1420000", sealedSecrets: SEAL }); + await service.openStored({ userId: "user-1", organizationId: null, dseq: "1420000", sealedSecrets: SEAL }); expect(secretCipherService.decrypt).toHaveBeenCalledWith({ userId: "user-1" }, SEAL, { sub: "user-1", dseq: "1420000" }); }); @@ -500,14 +501,14 @@ describe(SdlSecretsService.name, () => { const secrets = { TOKEN: faker.string.alphanumeric(32), DATABASE_URL: `postgres://app:${faker.string.alphanumeric(16)}@db/app` }; const { service } = setup({ stored: secrets }); - await expect(service.openStored({ userId: "user-1", dseq: "1420000", sealedSecrets: SEAL })).resolves.toEqual(secrets); + await expect(service.openStored({ userId: "user-1", organizationId: null, dseq: "1420000", sealedSecrets: SEAL })).resolves.toEqual(secrets); }); it("says nothing about a value in what it logs", async () => { const value = faker.string.alphanumeric(32); const { service, logger } = setup({ stored: { TOKEN: value } }); - await service.openStored({ userId: "user-1", dseq: "1420000", sealedSecrets: SEAL }); + await service.openStored({ userId: "user-1", organizationId: null, dseq: "1420000", sealedSecrets: SEAL }); expect(logger.info).toHaveBeenCalledWith({ event: "SDL_SECRETS_STORED_OPENED", userId: "user-1", dseq: "1420000", secretCount: 1 }); }); @@ -516,21 +517,21 @@ describe(SdlSecretsService.name, () => { const { service, secretCipherService } = setup(); secretCipherService.decrypt.mockResolvedValue(JSON.stringify({ TOKEN: { nested: "one" } })); - await expect(service.openStored({ userId: "user-1", dseq: "1420000", sealedSecrets: SEAL })).rejects.toMatchObject({ status: 500 }); + await expect(service.openStored({ userId: "user-1", organizationId: null, dseq: "1420000", sealedSecrets: SEAL })).rejects.toMatchObject({ status: 500 }); }); it("refuses a payload that is not json at all", async () => { const { service, secretCipherService } = setup(); secretCipherService.decrypt.mockResolvedValue("not-json"); - await expect(service.openStored({ userId: "user-1", dseq: "1420000", sealedSecrets: SEAL })).rejects.toMatchObject({ status: 500 }); + await expect(service.openStored({ userId: "user-1", organizationId: null, dseq: "1420000", sealedSecrets: SEAL })).rejects.toMatchObject({ status: 500 }); }); it("logs a payload it cannot read as the fault of the console that wrote it", async () => { const { service, secretCipherService, logger } = setup(); secretCipherService.decrypt.mockResolvedValue("[]"); - await expect(service.openStored({ userId: "user-1", dseq: "1420000", sealedSecrets: SEAL })).rejects.toThrow(); + await expect(service.openStored({ userId: "user-1", organizationId: null, dseq: "1420000", sealedSecrets: SEAL })).rejects.toThrow(); expect(logger.error).toHaveBeenCalledWith({ event: "SDL_SECRETS_STORED_PAYLOAD_INVALID", userId: "user-1", dseq: "1420000" }); }); @@ -539,14 +540,14 @@ describe(SdlSecretsService.name, () => { const { service, secretCipherService } = setup(); secretCipherService.decrypt.mockRejectedValue(refusal); - await expect(service.openStored({ userId: "user-1", dseq: "1420000", sealedSecrets: SEAL })).rejects.toBe(refusal); + await expect(service.openStored({ userId: "user-1", organizationId: null, dseq: "1420000", sealedSecrets: SEAL })).rejects.toBe(refusal); }); describe("a token sealed for another member than the one the row is filed under", () => { it("answers that the stored secrets can no longer be opened and that a full update stores them again", async () => { const { service } = setup(); - await expect(service.openStored({ userId: "user-2", dseq: "1420000", sealedSecrets: tokenWithHeader() })).rejects.toMatchObject({ + await expect(service.openStored({ userId: "user-2", organizationId: null, dseq: "1420000", sealedSecrets: tokenWithHeader() })).rejects.toMatchObject({ status: 409, errorCode: STORED_SECRETS_UNAVAILABLE_ERROR_CODE, message: expect.stringContaining("Update the deployment with its full SDL") @@ -556,7 +557,7 @@ describe(SdlSecretsService.name, () => { it("spends no key-service call on it and logs who it was opened for", async () => { const { service, secretCipherService, logger } = setup(); - await expect(service.openStored({ userId: "user-2", dseq: "1420000", sealedSecrets: tokenWithHeader() })).rejects.toThrow(); + await expect(service.openStored({ userId: "user-2", organizationId: null, dseq: "1420000", sealedSecrets: tokenWithHeader() })).rejects.toThrow(); expect(secretCipherService.decrypt).not.toHaveBeenCalled(); expect(logger.warn).toHaveBeenCalledWith({ event: "SDL_SECRETS_SEALED_FOR_ANOTHER_USER", userId: "user-2", dseq: "1420000" }); @@ -565,9 +566,59 @@ describe(SdlSecretsService.name, () => { it("opens a token sealed for the member the row is filed under", async () => { const { service, secretCipherService } = setup({ stored: { TOKEN: "one" } }); - await expect(service.openStored({ userId: "user-1", dseq: "1420000", sealedSecrets: tokenWithHeader() })).resolves.toEqual({ TOKEN: "one" }); + await expect(service.openStored({ userId: "user-1", organizationId: null, dseq: "1420000", sealedSecrets: tokenWithHeader() })).resolves.toEqual({ + TOKEN: "one" + }); expect(secretCipherService.decrypt).toHaveBeenCalled(); }); + + it("answers the same for a row whose member has deleted their account, spending no key-service call", async () => { + const { service, secretCipherService } = setup(); + + await expect(service.openStored({ userId: null, organizationId: "team-1", dseq: "1420000", sealedSecrets: tokenWithHeader() })).rejects.toMatchObject({ + status: 409, + errorCode: STORED_SECRETS_UNAVAILABLE_ERROR_CODE + }); + expect(secretCipherService.decrypt).not.toHaveBeenCalled(); + }); + }); + + describe("a token sealed under a team's key", () => { + it("opens it under the key of the team the row is filed in, bound to that team and the deployment", async () => { + const { service, secretCipherService } = setup({ stored: { TOKEN: "one" } }); + const sealedSecrets = tokenWithHeader({ org: "team-1", dseq: "1420000" }); + + await expect(service.openStored({ userId: "user-2", organizationId: "team-1", dseq: "1420000", sealedSecrets })).resolves.toEqual({ TOKEN: "one" }); + expect(secretCipherService.decrypt).toHaveBeenCalledExactlyOnceWith({ organizationId: "team-1" }, sealedSecrets, { org: "team-1", dseq: "1420000" }); + }); + + it("opens it for a row whose member has deleted their account", async () => { + const { service } = setup({ stored: { TOKEN: "one" } }); + const sealedSecrets = tokenWithHeader({ org: "team-1", dseq: "1420000" }); + + await expect(service.openStored({ userId: null, organizationId: "team-1", dseq: "1420000", sealedSecrets })).resolves.toEqual({ TOKEN: "one" }); + }); + + it("binds it to the row's team rather than to the one its header names", async () => { + const { service, secretCipherService } = setup({ stored: { TOKEN: "one" } }); + const sealedSecrets = tokenWithHeader({ org: "team-2", dseq: "1420000" }); + + await service.openStored({ userId: "user-1", organizationId: "team-1", dseq: "1420000", sealedSecrets }); + + expect(secretCipherService.decrypt).toHaveBeenCalledExactlyOnceWith({ organizationId: "team-1" }, sealedSecrets, { org: "team-1", dseq: "1420000" }); + }); + + it("refuses it on a row filed in no organization, as a token beyond any key, without a key-service call", async () => { + const { service, secretCipherService, logger } = setup(); + const sealedSecrets = tokenWithHeader({ org: "team-1", dseq: "1420000" }); + + await expect(service.openStored({ userId: "user-1", organizationId: null, dseq: "1420000", sealedSecrets })).rejects.toMatchObject({ + status: 500, + errorCode: SECRET_UNREADABLE_ERROR_CODE + }); + expect(secretCipherService.decrypt).not.toHaveBeenCalled(); + expect(logger.error).toHaveBeenCalledWith({ event: "SDL_SECRETS_SEALED_FOR_ORGANIZATION_OUTSIDE_ONE", userId: "user-1", dseq: "1420000" }); + }); }); describe("a token that will not decrypt", () => { @@ -575,7 +626,7 @@ describe(SdlSecretsService.name, () => { const { service, secretCipherService, logger } = setup(); secretCipherService.decrypt.mockRejectedValue(createError(500, "Unable to read stored secrets")); - await expect(service.openStored({ userId: "user-1", dseq: "1420000", sealedSecrets: tokenWithHeader() })).rejects.toThrow(); + await expect(service.openStored({ userId: "user-1", organizationId: null, dseq: "1420000", sealedSecrets: tokenWithHeader() })).rejects.toThrow(); expect(logger.error).toHaveBeenCalledWith({ event: "SECRET_DECRYPT_FAILED", @@ -590,7 +641,7 @@ describe(SdlSecretsService.name, () => { const sealedSecrets = tokenWithHeader(); secretCipherService.decrypt.mockRejectedValue(createError(500, "Unable to read stored secrets")); - await expect(service.openStored({ userId: "user-1", dseq: "1420000", sealedSecrets })).rejects.toThrow(); + await expect(service.openStored({ userId: "user-1", organizationId: null, dseq: "1420000", sealedSecrets })).rejects.toThrow(); const [[recorded]] = vi.mocked(logger.error).mock.calls; expect(JSON.stringify(recorded)).not.toContain(sealedSecrets.split(".")[3]); @@ -600,7 +651,7 @@ describe(SdlSecretsService.name, () => { const { service, secretCipherService, logger } = setup(); secretCipherService.decrypt.mockRejectedValue(createError(500, "Unable to read stored secrets")); - await expect(service.openStored({ userId: "user-1", dseq: "1420000", sealedSecrets: SEAL })).rejects.toThrow(); + await expect(service.openStored({ userId: "user-1", organizationId: null, dseq: "1420000", sealedSecrets: SEAL })).rejects.toThrow(); expect(logger.error).toHaveBeenCalledWith(expect.objectContaining({ event: "SECRET_DECRYPT_FAILED", claims: undefined })); }); @@ -609,14 +660,16 @@ describe(SdlSecretsService.name, () => { const { service, secretCipherService } = setup(); secretCipherService.decrypt.mockRejectedValue(createError(503, "Service temporarily unavailable")); - await expect(service.openStored({ userId: "user-1", dseq: "1420000", sealedSecrets: tokenWithHeader() })).rejects.toMatchObject({ status: 503 }); + await expect(service.openStored({ userId: "user-1", organizationId: null, dseq: "1420000", sealedSecrets: tokenWithHeader() })).rejects.toMatchObject({ + status: 503 + }); }); it("records nothing for a key service that is merely unreachable, which is no evidence of tampering", async () => { const { service, secretCipherService, logger } = setup(); secretCipherService.decrypt.mockRejectedValue(createError(503, "Service temporarily unavailable")); - await expect(service.openStored({ userId: "user-1", dseq: "1420000", sealedSecrets: tokenWithHeader() })).rejects.toThrow(); + await expect(service.openStored({ userId: "user-1", organizationId: null, dseq: "1420000", sealedSecrets: tokenWithHeader() })).rejects.toThrow(); expect(logger.error).not.toHaveBeenCalledWith(expect.objectContaining({ event: "SECRET_DECRYPT_FAILED" })); }); @@ -625,7 +678,7 @@ describe(SdlSecretsService.name, () => { const { service, secretCipherService, logger } = setup(); secretCipherService.decrypt.mockRejectedValue(new Error("boom")); - await expect(service.openStored({ userId: "user-1", dseq: "1420000", sealedSecrets: tokenWithHeader() })).rejects.toThrow(); + await expect(service.openStored({ userId: "user-1", organizationId: null, dseq: "1420000", sealedSecrets: tokenWithHeader() })).rejects.toThrow(); expect(logger.error).toHaveBeenCalledWith(expect.objectContaining({ event: "SECRET_DECRYPT_FAILED" })); }); @@ -711,8 +764,8 @@ describe(SdlSecretsService.name, () => { }); }); - function tokenWithHeader() { - const header = Buffer.from(JSON.stringify({ alg: "dir", enc: "A256GCM", kid: "data-key-1", sub: "user-1", dseq: "1420000" })).toString("base64url"); + function tokenWithHeader(claims: Record = { sub: "user-1", dseq: "1420000" }) { + const header = Buffer.from(JSON.stringify({ alg: "dir", enc: "A256GCM", kid: "data-key-1", ...claims })).toString("base64url"); return [header, "", "aXY", "Y2lwaGVydGV4dA", "dGFn"].join("."); } @@ -742,3 +795,23 @@ describe(SdlSecretsService.name, () => { return { service, unsealerService, secretCipherService, logger, config }; } }); + +describe(isSealedForOrganization.name, () => { + it("recognizes a token naming a team", () => { + expect(isSealedForOrganization(sealedWith({ org: "team-1", dseq: "1420000" }))).toBe(true); + }); + + it("recognizes a token naming a member as not sealed for a team", () => { + expect(isSealedForOrganization(sealedWith({ sub: "user-1", dseq: "1420000" }))).toBe(false); + }); + + it("treats a token whose header cannot be read as not sealed for a team", () => { + expect(isSealedForOrganization("not-a-token")).toBe(false); + }); + + function sealedWith(claims: Record) { + return [Buffer.from(JSON.stringify({ alg: "dir", enc: "A256GCM", kid: "data-key-1", ...claims })).toString("base64url"), "", "aXY", "Y3Q", "dGFn"].join( + "." + ); + } +}); diff --git a/apps/api/src/deployment/services/sdl-secrets/sdl-secrets.service.ts b/apps/api/src/deployment/services/sdl-secrets/sdl-secrets.service.ts index c3417712e7..06e5a21517 100644 --- a/apps/api/src/deployment/services/sdl-secrets/sdl-secrets.service.ts +++ b/apps/api/src/deployment/services/sdl-secrets/sdl-secrets.service.ts @@ -15,7 +15,9 @@ import { } from "@src/deployment/services/sdl-reference/sdl-reference.service"; import type { SdlSecrets } from "@src/deployment/services/sdl-secrets-unsealer/sdl-secrets-unsealer.service"; import { parseSdlSecrets, SdlSecretsUnsealerService } from "@src/deployment/services/sdl-secrets-unsealer/sdl-secrets-unsealer.service"; -import { SECRET_UNREADABLE_ERROR_MESSAGE } from "@src/secret/config/secret-at-rest.config"; +import { SECRET_UNREADABLE_ERROR_CODE, SECRET_UNREADABLE_ERROR_MESSAGE } from "@src/secret/config/secret-at-rest.config"; +import type { DataKeyOwner } from "@src/secret/repositories/data-key/data-key.repository"; +import type { SecretBinding } from "@src/secret/services/secret-cipher/secret-cipher.service"; import { SecretCipherService } from "@src/secret/services/secret-cipher/secret-cipher.service"; import { DeploymentConfigService } from "../deployment-config/deployment-config.service"; @@ -67,6 +69,21 @@ function claimsOf(sealedSecrets: string): Record | undefined { } } +/** Names the team a token was sealed for under the team's data key, where a token sealed under a member's key names the member as `sub`. */ +const ORGANIZATION_CLAIM = "org"; + +/** Says only which kind of key a token claims to be sealed under; opening it still holds the claim to the row it was read from. */ +export function isSealedForOrganization(sealedSecrets: string): boolean { + const claims = claimsOf(sealedSecrets); + + return claims !== undefined && Object.hasOwn(claims, ORGANIZATION_CLAIM); +} + +/** A stored token with the row it was read from; `userId` is the member the row is filed under, null once they deleted their account. */ +export type StoredSecretsOfDeployment = { userId: string | null; organizationId: string | null; dseq: string; sealedSecrets: string }; + +type Sealer = { owner: DataKeyOwner; binding: SecretBinding }; + export function unreferencedNameError(name: string): ValidationError { const echoed = name.slice(0, MAX_ECHOED_REFERENCE_LENGTH); @@ -153,10 +170,9 @@ export class SdlSecretsService { return supplied; } - /** Opens what `sealForStorage` wrote under the same binding, so a token moved to another deployment's row or another user's fails to open rather than resolving into it. */ - async openStored(input: { userId: string; dseq: string; sealedSecrets: string }): Promise { - this.#assertSealedForFiler(input); - const opened = await this.#decryptStored(input); + /** Opens what `sealForStorage` wrote under the same binding, so a token moved to another deployment's row, another user's or another organization's fails to open rather than resolving into it. */ + async openStored(input: StoredSecretsOfDeployment): Promise { + const opened = await this.#decryptStored(input, this.#sealerOf(input)); const secrets = parseSdlSecrets(opened); if (!secrets) { @@ -170,21 +186,42 @@ export class SdlSecretsService { return secrets; } - /** A team deployment taken over from a member who deleted their account still holds a token bound to that member, whose data key went with the account. */ - #assertSealedForFiler({ userId, dseq, sealedSecrets }: { userId: string; dseq: string; sealedSecrets: string }): void { + /** The key and binding come from the row rather than the token, whose header only says which kind of owner sealed it and is authenticated by the decrypt that follows. */ + #sealerOf(input: StoredSecretsOfDeployment): Sealer { + if (isSealedForOrganization(input.sealedSecrets)) { + return this.#organizationSealerOf(input); + } + + const userId = this.#assertSealedForFiler(input); + + return { owner: { userId }, binding: { sub: userId, dseq: input.dseq } }; + } + + #organizationSealerOf({ userId, organizationId, dseq }: StoredSecretsOfDeployment): Sealer { + if (!organizationId) { + this.#loggerService.error({ event: "SDL_SECRETS_SEALED_FOR_ORGANIZATION_OUTSIDE_ONE", userId, dseq }); + + throw createError(500, SECRET_UNREADABLE_ERROR_MESSAGE, { errorCode: SECRET_UNREADABLE_ERROR_CODE }); + } + + return { owner: { organizationId }, binding: { [ORGANIZATION_CLAIM]: organizationId, dseq } }; + } + + /** A team deployment taken over from a member who deleted their account may still hold a token bound to that member, whose data key went with the account. */ + #assertSealedForFiler({ userId, dseq, sealedSecrets }: StoredSecretsOfDeployment): string { const sealedFor = claimsOf(sealedSecrets)?.sub; - if (sealedFor === undefined || sealedFor === userId) return; + if (userId !== null && (sealedFor === undefined || sealedFor === userId)) return userId; this.#loggerService.warn({ event: "SDL_SECRETS_SEALED_FOR_ANOTHER_USER", userId, dseq }); throw createError(409, STORED_SECRETS_UNAVAILABLE_MESSAGE, { errorCode: STORED_SECRETS_UNAVAILABLE_ERROR_CODE }); } - /** Records only a permanent failure, and only the header's claims, which name the data key, the user and the deployment and carry none of the ciphertext. */ - async #decryptStored(input: { userId: string; dseq: string; sealedSecrets: string }): Promise { + /** Records only a permanent failure, and only the header's claims, which name the data key, the owner and the deployment and carry none of the ciphertext. */ + async #decryptStored(input: StoredSecretsOfDeployment, { owner, binding }: Sealer): Promise { try { - return await this.secretCipherService.decrypt({ userId: input.userId }, input.sealedSecrets, { sub: input.userId, dseq: input.dseq }); + return await this.secretCipherService.decrypt(owner, input.sealedSecrets, binding); } catch (error) { if (!isRetryable(error)) { this.#loggerService.error({ diff --git a/apps/api/src/secret/services/data-key-rekey/data-key-rekey.service.integration.ts b/apps/api/src/secret/services/data-key-rekey/data-key-rekey.service.integration.ts index 228de06653..f444d784c8 100644 --- a/apps/api/src/secret/services/data-key-rekey/data-key-rekey.service.integration.ts +++ b/apps/api/src/secret/services/data-key-rekey/data-key-rekey.service.integration.ts @@ -242,7 +242,10 @@ describe(`${DataKeyRekeyService.name} against Cloud KMS`, () => { return await inRequest(async () => Object.fromEntries( await Promise.all( - Object.entries(tokens).map(async ([dseq, sealedSecrets]) => [dseq, await sdlSecretsService.openStored({ userId, dseq, sealedSecrets })]) + Object.entries(tokens).map(async ([dseq, sealedSecrets]) => [ + dseq, + await sdlSecretsService.openStored({ userId, organizationId: null, dseq, sealedSecrets }) + ]) ) ) ); diff --git a/apps/api/src/secret/services/data-key-rekey/data-key-rekey.service.spec.ts b/apps/api/src/secret/services/data-key-rekey/data-key-rekey.service.spec.ts index 2a83345bd3..ed27076f9e 100644 --- a/apps/api/src/secret/services/data-key-rekey/data-key-rekey.service.spec.ts +++ b/apps/api/src/secret/services/data-key-rekey/data-key-rekey.service.spec.ts @@ -29,8 +29,14 @@ function tokenUnder(kid: string, nonce: string = randomUUID()) { return `${header}..iv.${Buffer.from(nonce).toString("base64url")}.tag`; } +function tokenSealedForTeam(organizationId: string) { + const header = Buffer.from(JSON.stringify({ org: organizationId, dseq: "888", alg: "dir", enc: "A256GCM", kid: FOREIGN_KEY_ID })).toString("base64url"); + + return `${header}..iv.ciphertext.tag`; +} + function deploymentSealedUnder(kid: string, dseq = String(1000 + Math.floor(Math.random() * 9000))): DeploymentStoredSecretsOfUser { - return { id: randomUUID(), dseq, sealedSecrets: tokenUnder(kid), updatedAt: new Date() }; + return { id: randomUUID(), dseq, organizationId: null, sealedSecrets: tokenUnder(kid), updatedAt: new Date() }; } describe(DataKeyRekeyService.name, () => { @@ -88,7 +94,7 @@ describe(DataKeyRekeyService.name, () => { const report = (await service.rekeyUser({ userId: USER_ID, dryRun: false })).unwrap(); - expect(sdlSecretsService.openStored).toHaveBeenCalledWith({ userId: USER_ID, dseq: "100", sealedSecrets: first.sealedSecrets }); + expect(sdlSecretsService.openStored).toHaveBeenCalledWith({ userId: USER_ID, organizationId: null, dseq: "100", sealedSecrets: first.sealedSecrets }); expect(sdlSecretsService.sealForStorage).toHaveBeenCalledWith({ userId: USER_ID, dseq: "100", secrets: TWO_SECRETS }); expect(deploymentSettingRepository.resealIfUnchanged).toHaveBeenCalledWith(first.id, first.sealedSecrets, resealedTokens()[0]); expect(report).toMatchObject({ deploymentsResealed: 2, secretsResealed: 4, activeDataKeyId: REPLACEMENT_KEY_ID, retiredDataKeyId: ACTIVE_KEY_ID }); @@ -205,6 +211,22 @@ describe(DataKeyRekeyService.name, () => { expect(dataKeyRepository.deleteRetired).not.toHaveBeenCalled(); }); + it("leaves a deployment sealed under its team's key to the team, counting it apart and still deleting the retired key", async () => { + const teamSealed = { ...deploymentSealedUnder(FOREIGN_KEY_ID, "888"), organizationId: "team-1", sealedSecrets: tokenSealedForTeam("team-1") }; + const { service, dataKeyRepository, sdlSecretsService } = setup({ deployments: [teamSealed], retireAgeMs: MIN_RETIREMENT_AGE_BEFORE_DELETE_MS * 2 }); + + const report = (await service.rekeyUser({ userId: USER_ID, dryRun: false })).unwrap(); + + expect(report).toMatchObject({ + deploymentsResealed: 0, + deploymentsSealedForOrganization: 1, + deploymentsUnderUnknownKey: [], + retiredDataKeyDeleted: true + }); + expect(sdlSecretsService.openStored).not.toHaveBeenCalled(); + expect(dataKeyRepository.deleteRetired).toHaveBeenCalled(); + }); + it("records a value that would not open, steps over it and fails the run at the end", async () => { const unreadable = deploymentSealedUnder(ACTIVE_KEY_ID, "300"); const readable = deploymentSealedUnder(ACTIVE_KEY_ID, "400"); @@ -259,7 +281,13 @@ describe(DataKeyRekeyService.name, () => { }); it("names a deployment whose token has no readable data key instead of counting it as already moved", async () => { - const unreadable: DeploymentStoredSecretsOfUser = { id: randomUUID(), dseq: "555", sealedSecrets: "not-a-token", updatedAt: new Date() }; + const unreadable: DeploymentStoredSecretsOfUser = { + id: randomUUID(), + dseq: "555", + organizationId: null, + sealedSecrets: "not-a-token", + updatedAt: new Date() + }; const { service, logger } = setup({ deployments: [unreadable, deploymentSealedUnder(ACTIVE_KEY_ID)] }); const result = await service.rekeyUser({ userId: USER_ID, dryRun: true }); diff --git a/apps/api/src/secret/services/data-key-rekey/data-key-rekey.service.ts b/apps/api/src/secret/services/data-key-rekey/data-key-rekey.service.ts index 6e8ccc6f41..295173d335 100644 --- a/apps/api/src/secret/services/data-key-rekey/data-key-rekey.service.ts +++ b/apps/api/src/secret/services/data-key-rekey/data-key-rekey.service.ts @@ -8,7 +8,7 @@ import { TxService } from "@src/core/services"; import { ExecutionContextService } from "@src/core/services/execution-context/execution-context.service"; import type { DeploymentStoredSecretsOfUser } from "@src/deployment/repositories/deployment-setting/deployment-setting.repository"; import { DeploymentSettingRepository } from "@src/deployment/repositories/deployment-setting/deployment-setting.repository"; -import { SdlSecretsService } from "@src/deployment/services/sdl-secrets/sdl-secrets.service"; +import { isSealedForOrganization, SdlSecretsService } from "@src/deployment/services/sdl-secrets/sdl-secrets.service"; import { SdlSecretsSealingKeyService } from "@src/deployment/services/sdl-secrets-sealing-key/sdl-secrets-sealing-key.service"; import type { SdlSecrets } from "@src/deployment/services/sdl-secrets-unsealer/sdl-secrets-unsealer.service"; import type { DataKeyOutput } from "@src/secret/repositories/data-key/data-key.repository"; @@ -36,6 +36,7 @@ export interface DataKeyRekeyReport { secretsResealed: number; deploymentsAlreadyUnderActiveKey: number; deploymentsMovedByAnotherWriter: number; + deploymentsSealedForOrganization: number; deploymentsUnderUnknownKey: string[]; retiredDataKeyDeleted: boolean; retiredDataKeyDeletableAfter: Date | null; @@ -55,6 +56,7 @@ interface ResealPass { secretsResealed: number; deploymentsAlreadyUnderActiveKey: number; deploymentsMovedByAnotherWriter: number; + deploymentsSealedForOrganization: number; deploymentsUnderUnknownKey: string[]; errors: unknown[]; } @@ -105,6 +107,7 @@ export class DataKeyRekeyService { secretsResealed: pass.secretsResealed, deploymentsAlreadyUnderActiveKey: pass.deploymentsAlreadyUnderActiveKey, deploymentsMovedByAnotherWriter: pass.deploymentsMovedByAnotherWriter, + deploymentsSealedForOrganization: pass.deploymentsSealedForOrganization, deploymentsUnderUnknownKey: pass.deploymentsUnderUnknownKey, retiredDataKeyDeleted: retirement.deleted, retiredDataKeyDeletableAfter: retirement.deletableAfter, @@ -183,6 +186,7 @@ export class DataKeyRekeyService { secretsResealed: 0, deploymentsAlreadyUnderActiveKey: 0, deploymentsMovedByAnotherWriter: 0, + deploymentsSealedForOrganization: 0, deploymentsUnderUnknownKey: [], errors: [] }; @@ -199,6 +203,12 @@ export class DataKeyRekeyService { } async #resealDeployment(userId: string, row: DeploymentStoredSecretsOfUser, keys: RekeyKeys, pass: ResealPass, dryRun: boolean) { + if (isSealedForOrganization(row.sealedSecrets)) { + pass.deploymentsSealedForOrganization += 1; + + return; + } + const kid = kidOf(row.sealedSecrets); if (kid !== undefined && kid === keys.targetId) { @@ -225,7 +235,7 @@ export class DataKeyRekeyService { /** Opened and sealed through the same paths a deployment uses, so the new token carries exactly the owner and deployment claims the old one did. */ async #reseal(userId: string, row: DeploymentStoredSecretsOfUser, pass: ResealPass, dryRun: boolean) { - const secrets = await this.sdlSecretsService.openStored({ userId, dseq: row.dseq, sealedSecrets: row.sealedSecrets }); + const secrets = await this.sdlSecretsService.openStored({ userId, organizationId: row.organizationId, dseq: row.dseq, sealedSecrets: row.sealedSecrets }); if (dryRun || (await this.#writeResealed(userId, row, secrets))) { pass.deploymentsResealed += 1; diff --git a/apps/api/test/functional/deployment-definition.spec.ts b/apps/api/test/functional/deployment-definition.spec.ts index b4fff13334..850d8fae69 100644 --- a/apps/api/test/functional/deployment-definition.spec.ts +++ b/apps/api/test/functional/deployment-definition.spec.ts @@ -295,7 +295,9 @@ describe("Deployment definitions", () => { return await container.resolve(ExecutionContextService).runWithContext(async () => { container.resolve(AuthService).currentUser = user; - return await container.resolve(SdlSecretsService).openStored({ userId: user.id, dseq: DSEQ, sealedSecrets: setting!.sealedSecrets! }); + return await container + .resolve(SdlSecretsService) + .openStored({ userId: user.id, organizationId: setting!.organizationId, dseq: DSEQ, sealedSecrets: setting!.sealedSecrets! }); }); } diff --git a/apps/api/test/functional/deployment-patch.spec.ts b/apps/api/test/functional/deployment-patch.spec.ts index 73b7cce028..5519c8fdc9 100644 --- a/apps/api/test/functional/deployment-patch.spec.ts +++ b/apps/api/test/functional/deployment-patch.spec.ts @@ -747,7 +747,9 @@ describe("PATCH /v1/deployments/{dseq}", () => { return await container.resolve(ExecutionContextService).runWithContext(async () => { container.resolve(AuthService).currentUser = user; - return await container.resolve(SdlSecretsService).openStored({ userId: user.id, dseq: DSEQ, sealedSecrets: setting!.sealedSecrets! }); + return await container + .resolve(SdlSecretsService) + .openStored({ userId: user.id, organizationId: setting!.organizationId, dseq: DSEQ, sealedSecrets: setting!.sealedSecrets! }); }); } diff --git a/apps/api/test/functional/deployment-sealed-secrets.spec.ts b/apps/api/test/functional/deployment-sealed-secrets.spec.ts index 2f12131465..cb759220b7 100644 --- a/apps/api/test/functional/deployment-sealed-secrets.spec.ts +++ b/apps/api/test/functional/deployment-sealed-secrets.spec.ts @@ -106,7 +106,10 @@ describe("Deployment sealed secrets", () => { findByUserId: async (id: string) => knownWallets[id], findOneByUserId: async (id: string) => knownWallets[id][0], findOneUsedBy: async (id: string) => knownWallets[id][0], - findById: async (id: number) => Object.values(knownWallets).flat().find(wallet => wallet.id === id) + findById: async (id: number) => + Object.values(knownWallets) + .flat() + .find(wallet => wallet.id === id) } as unknown as UserWalletRepository); vi.spyOn(signerService, "executeDecodedTxByUserWallet").mockResolvedValue({ code: 200, @@ -899,7 +902,7 @@ describe("Deployment sealed secrets", () => { return await executionContextService.runWithContext(async () => { authService.currentUser = user; - return await container.resolve(SdlSecretsService).openStored({ userId: user.id, dseq, sealedSecrets }); + return await container.resolve(SdlSecretsService).openStored({ userId: user.id, organizationId: null, dseq, sealedSecrets }); }); } diff --git a/apps/api/test/functional/deployments.spec.ts b/apps/api/test/functional/deployments.spec.ts index 1a5949cb9f..93d910dd5c 100644 --- a/apps/api/test/functional/deployments.spec.ts +++ b/apps/api/test/functional/deployments.spec.ts @@ -2873,7 +2873,7 @@ describe("Deployments API", () => { return await container.resolve(ExecutionContextService).runWithContext(async () => { container.resolve(AuthService).currentUser = mock({ id: user.id }); - return await container.resolve(SdlSecretsService).openStored({ userId: user.id, dseq, sealedSecrets: token }); + return await container.resolve(SdlSecretsService).openStored({ userId: user.id, organizationId: null, dseq, sealedSecrets: token }); }); } diff --git a/apps/api/test/functional/organization-wallet.spec.ts b/apps/api/test/functional/organization-wallet.spec.ts index a384913ee1..51bcaf18f3 100644 --- a/apps/api/test/functional/organization-wallet.spec.ts +++ b/apps/api/test/functional/organization-wallet.spec.ts @@ -296,7 +296,7 @@ describe("Organization wallet", () => { const response = await createInheriting("123", { authorization: `Bearer ${token}`, "x-organization-id": team.id }); - expect(openStored).toHaveBeenCalledWith({ userId: team.createdByUserId, dseq: "123", sealedSecrets: "sealed.token" }); + expect(openStored).toHaveBeenCalledWith({ userId: team.createdByUserId, organizationId: team.id, dseq: "123", sealedSecrets: "sealed.token" }); expect(response.status).toBe(402); }); }); From 0dc4a8f7e23156ee7650e95a9283baeef7052204 Mon Sep 17 00:00:00 2001 From: Maxime Beauchamp <15185355+baktun14@users.noreply.github.com> Date: Sat, 17 Oct 2026 23:09:59 -0700 Subject: [PATCH 2/2] feat(deployment): log team-sealed deployments in re-key batches and triage team tokens The re-key's batch log now carries the count of team deployments it leaves to the team's key, as its end report already did. The runbook names that report field and tells the decrypt-failure triage to compare a team token's kid with the organization's data keys rather than the member's. Part of CON-1213 --- apps/api/docs/runbooks/deployment-secrets.md | 30 +++++++++---------- .../data-key-rekey.service.spec.ts | 6 +++- .../data-key-rekey/data-key-rekey.service.ts | 1 + 3 files changed, 21 insertions(+), 16 deletions(-) diff --git a/apps/api/docs/runbooks/deployment-secrets.md b/apps/api/docs/runbooks/deployment-secrets.md index 72b7ba84e7..c8ca2f6af8 100644 --- a/apps/api/docs/runbooks/deployment-secrets.md +++ b/apps/api/docs/runbooks/deployment-secrets.md @@ -117,7 +117,7 @@ DEPLOYMENT_ENV=staging NETWORK=sandbox doppler run -p console-api -c staging-san node --require ./dist/instrumentation.js ./dist/console.js rekey-user-data-key --user-id ``` - The run retires the user's active key and inserts its replacement in one transaction, then re-seals each of the user's tokens under the new key. Both keys stay openable throughout and the user keeps deploying. The report states `deploymentsResealed`, `secretsResealed`, `deploymentsAlreadyUnderActiveKey`, `deploymentsMovedByAnotherWriter`, `deploymentsUnderUnknownKey`, `retiredDataKeyDeleted` and `retiredDataKeyDeletableAfter`. + The run retires the user's active key and inserts its replacement in one transaction, then re-seals each of the user's tokens under the new key. Both keys stay openable throughout and the user keeps deploying. The report states `deploymentsResealed`, `secretsResealed`, `deploymentsAlreadyUnderActiveKey`, `deploymentsMovedByAnotherWriter`, `deploymentsSealedForOrganization`, `deploymentsUnderUnknownKey`, `retiredDataKeyDeleted` and `retiredDataKeyDeletableAfter`. `deploymentsSealedForOrganization` counts the user's team deployments whose token is sealed under the team's own data key; the user's key never sealed them, so the re-key leaves them where they are and they do not hold back the retired key's deletion. 3. The first run always keeps the retired key and reports `retiredDataKeyDeleted: false`, because a request that read the old key just before it was retired may still be writing a value under it. Run the command again at or after `retiredDataKeyDeletableAfter`, a minute after the retirement. That run re-seals any straggler, checks that nothing is still sealed under the retired key, and deletes it. Only then is the leaked key useless. @@ -165,7 +165,7 @@ There is no threshold to tune: a healthy system produces none. The event is writ What it means: a token's ciphertext or header was altered, or it names a data key the user does not hold. Both are wrong in a way a bug is not. -What to look at first: the event carries `userId`, `dseq` and the token's header `claims`. Compare `claims.kid` with that user's `data_keys` rows. If a `DATA_KEY_REKEY_*` run for that user is in progress or just finished, and the kid is the key it retired, a request opened a token during the window; re-run the re-key. Otherwise treat it as tampering: snapshot the database, page security, and do not repair the row. +What to look at first: the event carries `userId`, `dseq` and the token's header `claims`. A token sealed under a team's data key has an `org` claim instead of `sub`: compare its `claims.kid` with that organization's `data_keys` rows (`organization_id = claims.org`), not the user's. A re-key only ever moves a user's own tokens, so a team token whose kid the team does not hold is tampering. For a user's token, compare `claims.kid` with that user's `data_keys` rows. If a `DATA_KEY_REKEY_*` run for that user is in progress or just finished, and the kid is the key it retired, a request opened a token during the window; re-run the re-key. Otherwise treat it as tampering: snapshot the database, page security, and do not repair the row. ### 5.2 Many distinct users unwrapped in a short window @@ -328,16 +328,16 @@ The same property is what makes section 2 step 5 load-bearing: destroying a KMS ## 8. Events and metrics -| Name | Kind | Meaning | -| ------------------------------------------------- | ------------ | ------------------------------------------------------------------------------------------------------------------------------------- | -| `USER_DATA_KEY_UNWRAPPED` | event, info | A request unwrapped a user's data key. Carries `userId`, `dataKeyId`, `kid`. The only per-user record of secret access. | -| `USER_DATA_KEY_WRAPPED_UNDER_UNKNOWN_KID` | event, error | A data key names a KMS version the console cannot resolve or the service refuses (disabled, destroyed, unknown). The user gets a 503. | -| `USER_DATA_KEY_UNWRAP_FAILED` | event, error | The key service was unreachable, answered with an error, or answered nonsense; `failure` says which. The user gets a 503. | -| `USER_DATA_KEY_UNREADABLE` | event, error | The wrapped key itself is corrupt. Permanent. | -| `SECRET_DECRYPT_FAILED` | event, error | A stored token would not open. Section 5.1. | -| `SDL_SECRETS_SEALED`, `SDL_SECRETS_STORED_OPENED` | event, info | A token was written or read for a deployment. | -| `DATA_KEY_REWRAP_*` | events | The scheduled rotation's start, batches, refusals, failures and end report. | -| `DATA_KEY_REKEY_*` | events | The per-user re-key's start, retirement, batches, kept or deleted retired key, and end report. | -| `kms_key_service_calls_total{status,failure}` | counter | Key-service unwraps by outcome. | -| `kms_key_service_call_duration_ms` | histogram | Key-service latency. | -| `user_data_key_unwraps_per_request` | histogram | Unwraps a request spent on one user's key. Expected 1; 2 during that user's re-key window. | +| Name | Kind | Meaning | +| ------------------------------------------------- | ------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `USER_DATA_KEY_UNWRAPPED` | event, info | A request unwrapped a data key. Carries `userId`, or `organizationId` for a team's key, plus `dataKeyId` and `kid`. The only per-owner record of secret access. | +| `USER_DATA_KEY_WRAPPED_UNDER_UNKNOWN_KID` | event, error | A data key names a KMS version the console cannot resolve or the service refuses (disabled, destroyed, unknown). The user gets a 503. | +| `USER_DATA_KEY_UNWRAP_FAILED` | event, error | The key service was unreachable, answered with an error, or answered nonsense; `failure` says which. The user gets a 503. | +| `USER_DATA_KEY_UNREADABLE` | event, error | The wrapped key itself is corrupt. Permanent. | +| `SECRET_DECRYPT_FAILED` | event, error | A stored token would not open. Section 5.1. | +| `SDL_SECRETS_SEALED`, `SDL_SECRETS_STORED_OPENED` | event, info | A token was written or read for a deployment. | +| `DATA_KEY_REWRAP_*` | events | The scheduled rotation's start, batches, refusals, failures and end report. | +| `DATA_KEY_REKEY_*` | events | The per-user re-key's start, retirement, batches, kept or deleted retired key, and end report. | +| `kms_key_service_calls_total{status,failure}` | counter | Key-service unwraps by outcome. | +| `kms_key_service_call_duration_ms` | histogram | Key-service latency. | +| `user_data_key_unwraps_per_request` | histogram | Unwraps a request spent on one user's key. Expected 1; 2 during that user's re-key window. | diff --git a/apps/api/src/secret/services/data-key-rekey/data-key-rekey.service.spec.ts b/apps/api/src/secret/services/data-key-rekey/data-key-rekey.service.spec.ts index ed27076f9e..45f8d1db26 100644 --- a/apps/api/src/secret/services/data-key-rekey/data-key-rekey.service.spec.ts +++ b/apps/api/src/secret/services/data-key-rekey/data-key-rekey.service.spec.ts @@ -213,7 +213,10 @@ describe(DataKeyRekeyService.name, () => { it("leaves a deployment sealed under its team's key to the team, counting it apart and still deleting the retired key", async () => { const teamSealed = { ...deploymentSealedUnder(FOREIGN_KEY_ID, "888"), organizationId: "team-1", sealedSecrets: tokenSealedForTeam("team-1") }; - const { service, dataKeyRepository, sdlSecretsService } = setup({ deployments: [teamSealed], retireAgeMs: MIN_RETIREMENT_AGE_BEFORE_DELETE_MS * 2 }); + const { service, dataKeyRepository, sdlSecretsService, logger } = setup({ + deployments: [teamSealed], + retireAgeMs: MIN_RETIREMENT_AGE_BEFORE_DELETE_MS * 2 + }); const report = (await service.rekeyUser({ userId: USER_ID, dryRun: false })).unwrap(); @@ -223,6 +226,7 @@ describe(DataKeyRekeyService.name, () => { deploymentsUnderUnknownKey: [], retiredDataKeyDeleted: true }); + expect(logger.info).toHaveBeenCalledWith(expect.objectContaining({ event: "DATA_KEY_REKEY_BATCH", sealedForOrganization: 1, resealed: 0 })); expect(sdlSecretsService.openStored).not.toHaveBeenCalled(); expect(dataKeyRepository.deleteRetired).toHaveBeenCalled(); }); diff --git a/apps/api/src/secret/services/data-key-rekey/data-key-rekey.service.ts b/apps/api/src/secret/services/data-key-rekey/data-key-rekey.service.ts index 295173d335..f576b2bb99 100644 --- a/apps/api/src/secret/services/data-key-rekey/data-key-rekey.service.ts +++ b/apps/api/src/secret/services/data-key-rekey/data-key-rekey.service.ts @@ -311,6 +311,7 @@ export class DataKeyRekeyService { resealed: pass.deploymentsResealed, alreadyUnderActiveKey: pass.deploymentsAlreadyUnderActiveKey, movedByAnotherWriter: pass.deploymentsMovedByAnotherWriter, + sealedForOrganization: pass.deploymentsSealedForOrganization, underUnknownKey: pass.deploymentsUnderUnknownKey.length, failed: pass.errors.length };