From 0cf64d36a5ca37d3e4da86c3354c6633d316b612 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 27 Jul 2026 19:08:07 -0500 Subject: [PATCH] feat(secrets): write through external values and deep-link details (#10196) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source control plane people use to manage AI-agent companies. > - Its secrets subsystem can resolve external references such as AWS Secrets Manager values without copying those values into Paperclip custody. > - Operators also need to rotate a referenced secret's value while preserving the same provider reference for consumers inside and outside Paperclip. > - Previously, external-reference rotation could only retarget metadata, and secret detail sheets were driven by local component state rather than shareable navigation state. > - This pull request adds an optional provider write capability, implements AWS Secrets Manager write-through rotation, and exposes capability-aware rotate modes in the UI. > - It also makes secret and each-user definition detail sheets URL-driven and adds a copy-link action. > - The benefit is that operators can update the canonical external value safely while keeping AWS rotation tracking intact, and they can share or navigate directly to secret details. ## Linked Issues or Issue Description ### Subsystem affected Cross-cutting (`server/`, `ui/`, and `packages/shared`). ### Problem or motivation External-reference secrets can follow a provider-managed value, but Paperclip could not write a replacement value back to providers that support it. Operators had to leave Paperclip, update the value separately, and then return without an auditable Paperclip rotation record. Secret detail sheets also could not be shared or restored through browser history because their selection lived only in component state. ### Proposed solution Add an optional `updateExternalSecretValue` provider capability and surface it as `supportsExternalValueWrites`. Implement AWS writes with `PutSecretValue` while leaving the resolution `versionId` unset so future reads continue following `AWSCURRENT`. Add write-value and retarget modes to the rotate dialog for capable providers. Drive secret detail selection from `?secret=` / `?definition=` query parameters and provide a copy-link action. ### Alternatives considered Converting an external reference into a Paperclip-managed secret would break consumers that depend on the existing provider reference. Pinning reads to the newly written AWS version would prevent later out-of-band rotations from flowing through. Keeping sheet selection only in React state would not support browser Back or shareable links. ### Roadmap alignment This extends the completed “Secrets Manager with per-agent access” roadmap capability; it does not duplicate a separate planned roadmap item. Public GitHub searches found no duplicate or closely related issue or PR. ### Additional context The PR includes focused provider, service, and UI render coverage. Cutter also generated previews for the deep-linked detail sheet and capability-aware rotate modes. ## What Changed - Added optional external-value write support to the secret provider contract and provider descriptors. - Implemented AWS Secrets Manager write-through with `PutSecretValue`, audit material, and compensation when persistence fails after the provider write. - Allowed `secretService.rotate()` value updates for external references while rejecting ambiguous value-plus-retarget combinations and unsupported providers. - Added capability-aware “Write new value” and “Change reference” rotate modes with updated custody and action copy. - Made secret and each-user definition detail sheets source their selection from URL query parameters, compose with folder paths, close through browser history, and expose a copy-link action. - Added provider, service, and UI render coverage for write-through, rollback, capability messaging, dialog modes, and deep links. ## Verification - `pnpm vitest run server/src/__tests__/aws-secrets-manager-provider.test.ts` — 18 passed. - `pnpm vitest run server/src/__tests__/secrets-service.test.ts` — 75 passed. - `pnpm vitest run ui/src/pages/Secrets.render.test.tsx` — 31 passed. - `pnpm --filter @paperclipai/shared typecheck` — passed. - `pnpm --filter @paperclipai/server typecheck` — passed. - `pnpm --filter @paperclipai/ui typecheck` — passed. - `pnpm check:token-gates` — passed with all gates clean. ## Risks - External value writes affect the canonical provider secret and therefore all consumers of that AWS secret; the UI explicitly labels this custody behavior. - A provider write can succeed before Paperclip persistence fails. The service records the written version and AWS support includes compensation coverage to restore the prior value where possible; unrecoverable failures return explicit audit-safe error details. - URL-driven sheet state changes navigation behavior; render tests cover deep links, Back/close behavior, and composition with folder query state. - No database migration or breaking API requirement is introduced; providers without the optional capability retain reference-only behavior. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex using exact model ID `gpt-5.6-sol`, high reasoning mode, Codex CLI `0.142.5`, with repository, shell, Git, GitHub CLI, and code-execution tools. The runtime did not expose a context-window size. - Earlier implementation commits were assisted by Anthropic `Claude Fable 5` as recorded in their commit trailers; the exact backend model ID and context-window size were not preserved in the workspace metadata. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes, or confirmed no documentation change is required - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Claude Fable 5 Co-authored-by: Paperclip --- packages/shared/src/types/secrets.ts | 1 + .../aws-secrets-manager-provider.test.ts | 147 +++++++++++++++++ server/src/__tests__/secrets-service.test.ts | 150 ++++++++++++++++++ .../secrets/aws-secrets-manager-provider.ts | 64 +++++++- server/src/secrets/types.ts | 6 + server/src/services/secrets.ts | 35 +++- ui/src/pages/Secrets.render.test.tsx | 140 +++++++++++++++- ui/src/pages/Secrets.tsx | 138 ++++++++++++---- 8 files changed, 637 insertions(+), 44 deletions(-) diff --git a/packages/shared/src/types/secrets.ts b/packages/shared/src/types/secrets.ts index 3d754476da5..47e16022b76 100644 --- a/packages/shared/src/types/secrets.ts +++ b/packages/shared/src/types/secrets.ts @@ -128,6 +128,7 @@ export interface SecretProviderDescriptor { requiresExternalRef: boolean; supportsManagedValues?: boolean; supportsExternalReferences?: boolean; + supportsExternalValueWrites?: boolean; configured?: boolean; } diff --git a/server/src/__tests__/aws-secrets-manager-provider.test.ts b/server/src/__tests__/aws-secrets-manager-provider.test.ts index 1b0e60d05a2..c2fc4cb6e17 100644 --- a/server/src/__tests__/aws-secrets-manager-provider.test.ts +++ b/server/src/__tests__/aws-secrets-manager-provider.test.ts @@ -386,6 +386,153 @@ describe("awsSecretsManagerProvider", () => { ).rejects.toThrow(/Paperclip-managed namespace/i); }); + it("writes new values through to externally referenced AWS secrets as AWSCURRENT", async () => { + const calls: Array<{ op: string; input: Record }> = []; + const provider = createAwsSecretsManagerProvider({ + config: { + region: "us-east-1", + endpoint: "https://secretsmanager.us-east-1.amazonaws.com", + deploymentId: "prod-use1", + prefix: "paperclip", + kmsKeyId: "arn:aws:kms:us-east-1:123456789012:key/test", + environmentTag: "production", + providerOwnerTag: "paperclip", + deleteRecoveryWindowDays: 30, + }, + gateway: { + async createSecret() { + throw new Error("not used"); + }, + async putSecretValue(input) { + calls.push({ op: "putSecretValue", input }); + return { + ARN: "arn:aws:secretsmanager:us-east-1:123456789012:secret:shared/external", + VersionId: "aws-version-9", + }; + }, + async getSecretValue(input) { + calls.push({ op: "getSecretValue", input }); + return { VersionId: "aws-version-8", SecretString: "previous-value" }; + }, + async deleteSecret() { + throw new Error("not used"); + }, + }, + }); + + const prepared = await provider.updateExternalSecretValue!({ + externalRef: "arn:aws:secretsmanager:us-east-1:123456789012:secret:shared/external", + value: "written-through-value", + context: { + companyId: "company-1", + secretKey: "neon-admin-api-key", + secretName: "paperclip-cloud/prod/provider/neon/admin-api-key", + version: 2, + }, + }); + + // No VersionStages: the write must become AWSCURRENT for all consumers. + expect(calls).toEqual([ + { + op: "getSecretValue", + input: { + SecretId: "arn:aws:secretsmanager:us-east-1:123456789012:secret:shared/external", + VersionStage: "AWSCURRENT", + }, + }, + { + op: "putSecretValue", + input: { + SecretId: "arn:aws:secretsmanager:us-east-1:123456789012:secret:shared/external", + SecretString: "written-through-value", + }, + }, + ]); + expect(JSON.stringify(prepared.material)).not.toContain("written-through-value"); + expect(prepared.externalRef).toBe( + "arn:aws:secretsmanager:us-east-1:123456789012:secret:shared/external", + ); + // Resolution keeps tracking AWSCURRENT rather than pinning the written version. + expect(prepared.providerVersionRef).toBeNull(); + expect(prepared.material.versionId).toBeNull(); + expect(prepared.material.lastWrittenVersionId).toBe("aws-version-9"); + expect(prepared.material.previousCurrentVersionId).toBe("aws-version-8"); + expect(prepared.material.source).toBe("external_reference"); + }); + + it("rejects external value writes under the Paperclip-managed namespace", async () => { + const provider = createAwsSecretsManagerProvider({ + config: { + region: "us-east-1", + endpoint: "https://secretsmanager.us-east-1.amazonaws.com", + deploymentId: "prod-use1", + prefix: "paperclip", + kmsKeyId: null, + environmentTag: "production", + providerOwnerTag: "paperclip", + deleteRecoveryWindowDays: 30, + }, + }); + + await expect( + provider.updateExternalSecretValue!({ + externalRef: + "arn:aws:secretsmanager:us-east-1:123456789012:secret:paperclip/prod-use1/company-2/openai-api-key", + value: "new-value", + }), + ).rejects.toThrow(/Paperclip-managed namespace/i); + }); + + it("restores the previous AWSCURRENT version when an external value write is rolled back", async () => { + const calls: Array<{ op: string; input: Record }> = []; + const provider = createAwsSecretsManagerProvider({ + config: { + region: "us-east-1", + endpoint: "https://secretsmanager.us-east-1.amazonaws.com", + deploymentId: "prod-use1", + prefix: "paperclip", + kmsKeyId: null, + environmentTag: "production", + providerOwnerTag: "paperclip", + deleteRecoveryWindowDays: 30, + }, + gateway: { + async createSecret() { throw new Error("not used"); }, + async putSecretValue() { throw new Error("not used"); }, + async getSecretValue() { throw new Error("not used"); }, + async deleteSecret() { throw new Error("not used"); }, + async updateSecretVersionStage(input) { + calls.push({ op: "updateSecretVersionStage", input }); + }, + }, + }); + + await provider.deleteOrArchive({ + material: { + scheme: "aws_secrets_manager_v1", + secretId: "arn:aws:secretsmanager:us-east-1:123456789012:secret:shared/external", + versionId: null, + source: "external_reference", + lastWrittenVersionId: "aws-version-9", + previousCurrentVersionId: "aws-version-8", + }, + externalRef: "arn:aws:secretsmanager:us-east-1:123456789012:secret:shared/external", + mode: "archive", + }); + + expect(calls).toEqual([ + { + op: "updateSecretVersionStage", + input: { + SecretId: "arn:aws:secretsmanager:us-east-1:123456789012:secret:shared/external", + VersionStage: "AWSCURRENT", + MoveToVersionId: "aws-version-8", + RemoveFromVersionId: "aws-version-9", + }, + }, + ]); + }); + it("lists remote AWS secrets with metadata only and never resolves plaintext", async () => { const calls: Array<{ op: string; input: Record }> = []; const provider = createAwsSecretsManagerProvider({ diff --git a/server/src/__tests__/secrets-service.test.ts b/server/src/__tests__/secrets-service.test.ts index b025ca3779f..6279a95e18d 100644 --- a/server/src/__tests__/secrets-service.test.ts +++ b/server/src/__tests__/secrets-service.test.ts @@ -2767,6 +2767,156 @@ describeEmbeddedPostgres("secretService", () => { expect(thrown instanceof Error ? thrown.message : String(thrown)).not.toContain("arn:aws"); }); + it("writes external reference rotations through the provider when a value is given", async () => { + const companyId = await seedCompany(); + const svc = secretService(db); + const awsVault = await svc.createProviderConfig(companyId, { + provider: "aws_secrets_manager", + displayName: "AWS production", + config: { region: "us-east-1", namespace: "prod-use1" }, + }); + const externalRef = "arn:aws:secretsmanager:us-east-1:123456789012:secret:shared/neon-admin"; + const secret = await svc.create(companyId, { + name: `external-${randomUUID()}`, + provider: "aws_secrets_manager", + providerConfigId: awsVault.id, + managedMode: "external_reference", + externalRef, + }); + + const writeSpy = vi + .spyOn(awsSecretsManagerProvider, "updateExternalSecretValue") + .mockResolvedValueOnce({ + material: { + scheme: "aws_secrets_manager_v1", + secretId: externalRef, + versionId: null, + source: "external_reference", + lastWrittenVersionId: "aws-version-2", + }, + valueSha256: "a".repeat(64), + fingerprintSha256: "a".repeat(64), + externalRef, + providerVersionRef: null, + }); + const linkSpy = vi.spyOn(awsSecretsManagerProvider, "linkExternalSecret"); + + const rotated = await svc.rotate(secret.id, { value: "new-admin-key" }, { userId: "user-1" }); + + expect(writeSpy).toHaveBeenCalledTimes(1); + expect(writeSpy.mock.calls[0]?.[0]).toMatchObject({ + externalRef, + value: "new-admin-key", + }); + expect(linkSpy).not.toHaveBeenCalled(); + expect(rotated.latestVersion).toBe(2); + expect(rotated.externalRef).toBe(externalRef); + + const versions = await db + .select() + .from(companySecretVersions) + .where(eq(companySecretVersions.secretId, secret.id)); + const current = versions.find((row) => row.status === "current"); + expect(current?.version).toBe(2); + expect(current?.providerVersionRef).toBeNull(); + expect(JSON.stringify(current)).not.toContain("new-admin-key"); + }); + + it("restores the provider current version when external value rotation persistence fails", async () => { + const companyId = await seedCompany(); + const svc = secretService(db); + const awsVault = await svc.createProviderConfig(companyId, { + provider: "aws_secrets_manager", + displayName: "AWS production", + config: { region: "us-east-1", namespace: "prod-use1" }, + }); + const externalRef = "arn:aws:secretsmanager:us-east-1:123456789012:secret:shared/rollback"; + const secret = await svc.create(companyId, { + name: "External rollback", + key: "external-rollback", + provider: "aws_secrets_manager", + providerConfigId: awsVault.id, + managedMode: "external_reference", + externalRef, + }); + const prepared = { + material: { + scheme: "aws_secrets_manager_v1", + secretId: externalRef, + versionId: null, + source: "external_reference", + lastWrittenVersionId: "aws-version-2", + previousCurrentVersionId: "aws-version-1", + }, + valueSha256: "a".repeat(64), + fingerprintSha256: "a".repeat(64), + externalRef, + providerVersionRef: null, + }; + vi.spyOn(awsSecretsManagerProvider, "updateExternalSecretValue").mockResolvedValueOnce(prepared); + const rollbackSpy = vi.spyOn(awsSecretsManagerProvider, "deleteOrArchive").mockResolvedValue(); + vi.spyOn(db, "transaction").mockRejectedValueOnce(new Error("db rotate failed")); + + await expect(svc.rotate(secret.id, { value: "new-value" })).rejects.toThrow( + "db rotate failed", + ); + + expect(rollbackSpy).toHaveBeenCalledWith(expect.objectContaining({ + material: prepared.material, + externalRef, + mode: "archive", + providerConfig: expect.objectContaining({ id: awsVault.id }), + context: { + companyId, + secretKey: "external-rollback", + secretName: "External rollback", + version: 2, + }, + })); + }); + + it("rejects external value rotations that also retarget or pin versions", async () => { + const companyId = await seedCompany(); + const svc = secretService(db); + const awsVault = await svc.createProviderConfig(companyId, { + provider: "aws_secrets_manager", + displayName: "AWS production", + config: { region: "us-east-1", namespace: "prod-use1" }, + }); + const secret = await svc.create(companyId, { + name: `external-${randomUUID()}`, + provider: "aws_secrets_manager", + providerConfigId: awsVault.id, + managedMode: "external_reference", + externalRef: "arn:aws:secretsmanager:us-east-1:123456789012:secret:shared/neon-admin", + }); + + await expect( + svc.rotate(secret.id, { + value: "new-admin-key", + externalRef: "arn:aws:secretsmanager:us-east-1:123456789012:secret:shared/other", + }), + ).rejects.toThrow(/not both/); + await expect( + svc.rotate(secret.id, { value: "new-admin-key", providerVersionRef: "pinned-1" }), + ).rejects.toThrow(/cannot pin/i); + }); + + it("rejects external value rotations when the provider cannot write values", async () => { + const companyId = await seedCompany(); + const svc = secretService(db); + const secret = await svc.create(companyId, { + name: `external-${randomUUID()}`, + provider: "vault", + managedMode: "external_reference", + externalRef: "kv/data/shared/neon-admin", + }); + + await expect(svc.rotate(secret.id, { value: "new-admin-key" })).rejects.toThrow( + /does not support writing values/, + ); + }); + it("imports AWS remote references row-by-row without fetching plaintext", async () => { const companyId = await seedCompany(); const svc = secretService(db); diff --git a/server/src/secrets/aws-secrets-manager-provider.ts b/server/src/secrets/aws-secrets-manager-provider.ts index a697556a169..87090c86c53 100644 --- a/server/src/secrets/aws-secrets-manager-provider.ts +++ b/server/src/secrets/aws-secrets-manager-provider.ts @@ -36,6 +36,8 @@ interface AwsSecretsManagerMaterial extends StoredSecretVersionMaterial { secretId: string; versionId: string | null; source: "managed" | "external_reference"; + lastWrittenVersionId?: string | null; + previousCurrentVersionId?: string | null; } interface AwsSecretsManagerConfig { @@ -264,6 +266,7 @@ function configuredAwsSecretsManagerDescriptor() { requiresExternalRef: false, supportsManagedValues: true, supportsExternalReferences: true, + supportsExternalValueWrites: true, configured: canLoadAwsSecretsManagerConfig(), }; } @@ -1179,6 +1182,42 @@ export function createAwsSecretsManagerProvider( assertNotManagedNamespaceExternalRef(config, input.externalRef); return createExternalReferenceMaterial(input.externalRef, input.providerVersionRef ?? null); }, + async updateExternalSecretValue(input) { + const config = resolveConfig(input.providerConfig); + assertNotManagedNamespaceExternalRef(config, input.externalRef); + const gateway = resolveGateway(config); + const secretId = input.externalRef.trim(); + + try { + const previous = await gateway.getSecretValue({ + SecretId: secretId, + VersionStage: DEFAULT_VERSION_STAGE, + }); + // No VersionStages: the new version becomes AWSCURRENT so every consumer of the + // referenced secret (not just Paperclip) picks up the new value. + const created = await gateway.putSecretValue({ + SecretId: secretId, + SecretString: input.value, + }); + const normalizedSecretId = created.ARN ?? created.Name ?? secretId; + // Material keeps versionId null so resolution keeps tracking AWSCURRENT and + // out-of-band rotations done directly in AWS still flow through. + const prepared = createExternalReferenceMaterial(normalizedSecretId, null); + const valueSha256 = sha256Hex(input.value); + return { + ...prepared, + material: { + ...prepared.material, + lastWrittenVersionId: created.VersionId ?? null, + previousCurrentVersionId: previous.VersionId ?? null, + }, + valueSha256, + fingerprintSha256: valueSha256, + }; + } catch (error) { + normalizeAwsError("updateExternalSecretValue", error); + } + }, async listRemoteSecrets(input): Promise { const config = resolveConfig(input.providerConfig); const gateway = resolveGateway(config); @@ -1277,10 +1316,31 @@ export function createAwsSecretsManagerProvider( ? asAwsSecretsManagerMaterial(input.material) : null; - if (material?.source !== "managed") return; - const config = resolveConfig(input.providerConfig); const gateway = resolveGateway(config); + + if (material?.source === "external_reference") { + if ( + input.mode === "archive" && + material.lastWrittenVersionId && + material.previousCurrentVersionId && + gateway.updateSecretVersionStage + ) { + try { + await gateway.updateSecretVersionStage({ + SecretId: material.secretId, + VersionStage: DEFAULT_VERSION_STAGE, + MoveToVersionId: material.previousCurrentVersionId, + RemoveFromVersionId: material.lastWrittenVersionId, + }); + } catch (error) { + normalizeAwsError("updateSecretVersionStage", error); + } + } + return; + } + if (material?.source !== "managed") return; + const secretId = resolveManagedSecretRef({ config, context: input.context, diff --git a/server/src/secrets/types.ts b/server/src/secrets/types.ts index 9edf7766be1..19ffd7359cd 100644 --- a/server/src/secrets/types.ts +++ b/server/src/secrets/types.ts @@ -150,6 +150,12 @@ export interface SecretProviderModule { context?: SecretProviderWriteContext; providerConfig?: SecretProviderVaultRuntimeConfig | null; }): Promise; + updateExternalSecretValue?(input: { + externalRef: string; + value: string; + context?: SecretProviderWriteContext; + providerConfig?: SecretProviderVaultRuntimeConfig | null; + }): Promise; listRemoteSecrets?(input: { providerConfig?: SecretProviderVaultRuntimeConfig | null; query?: string | null; diff --git a/server/src/services/secrets.ts b/server/src/services/secrets.ts index 3c2c8210056..b59f3a73b61 100644 --- a/server/src/services/secrets.ts +++ b/server/src/services/secrets.ts @@ -3606,6 +3606,27 @@ export function secretService(db: Db) { providerConfigId, }); const nextVersion = secret.latestVersion + 1; + const externalValueWrite = + secret.managedMode === "external_reference" && Boolean(input.value?.trim()); + if (externalValueWrite) { + const currentRef = secret.externalRef?.trim(); + if (!currentRef) { + throw unprocessable("External reference secrets require externalRef"); + } + if (input.externalRef?.trim() && input.externalRef.trim() !== currentRef) { + throw unprocessable( + "Provide either a new value or a new external reference, not both", + ); + } + if (input.providerVersionRef?.trim()) { + throw unprocessable("Value updates cannot pin providerVersionRef"); + } + if (!provider.updateExternalSecretValue) { + throw unprocessable( + `${provider.descriptor().label} does not support writing values to external reference secrets`, + ); + } + } if (secret.managedMode === "external_reference" && !(input.externalRef ?? secret.externalRef)?.trim()) { throw unprocessable("External reference secrets require externalRef"); } @@ -3623,8 +3644,14 @@ export function secretService(db: Db) { }; let prepared: PreparedSecretVersion; try { - prepared = - secret.managedMode === "external_reference" + prepared = externalValueWrite + ? await provider.updateExternalSecretValue!({ + externalRef: secret.externalRef ?? "", + value: input.value ?? "", + providerConfig, + context: providerWriteContext, + }) + : secret.managedMode === "external_reference" ? await provider.linkExternalSecret({ externalRef: input.externalRef ?? secret.externalRef ?? "", providerVersionRef: input.providerVersionRef ?? null, @@ -3660,7 +3687,7 @@ export function secretService(db: Db) { createdByUserId: actor?.userId ?? null, }); } catch (error) { - if (secret.managedMode !== "external_reference") { + if (secret.managedMode !== "external_reference" || externalValueWrite) { await cleanupPreparedProviderWrite({ provider, prepared, @@ -3707,7 +3734,7 @@ export function secretService(db: Db) { return updated; }); } catch (error) { - if (secret.managedMode !== "external_reference") { + if (secret.managedMode !== "external_reference" || externalValueWrite) { const cleaned = await cleanupPreparedProviderWrite({ provider, prepared, diff --git a/ui/src/pages/Secrets.render.test.tsx b/ui/src/pages/Secrets.render.test.tsx index d96f4bf4687..05fcf192076 100644 --- a/ui/src/pages/Secrets.render.test.tsx +++ b/ui/src/pages/Secrets.render.test.tsx @@ -1,6 +1,6 @@ // @vitest-environment jsdom -import { createRoot } from "react-dom/client"; +import { createRoot as createReactRoot, type Root } from "react-dom/client"; import { flushSync } from "react-dom"; import { MemoryRouter } from "react-router-dom"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; @@ -114,6 +114,7 @@ const providers: SecretProviderDescriptor[] = [ requiresExternalRef: false, supportsManagedValues: true, supportsExternalReferences: true, + supportsExternalValueWrites: true, configured: true, }, { @@ -159,12 +160,29 @@ const providerConfigs = [ }, ] satisfies Partial[]; +const activeRoots = new Set(); + async function act(callback: () => void | Promise) { - let result: void | Promise = undefined; + let result: void | Promise; flushSync(() => { result = callback(); }); - await result; + await result!; +} + +function createRoot(container: Element | DocumentFragment) { + const root = createReactRoot(container); + const unmount = root.unmount.bind(root); + root.unmount = () => { + if (!activeRoots.delete(root)) return; + unmount(); + }; + activeRoots.add(root); + return root; +} + +function unmountActiveRoots() { + for (const root of [...activeRoots]) root.unmount(); } async function flushReact() { @@ -368,6 +386,7 @@ describe("Secrets page layout", () => { }); afterEach(() => { + unmountActiveRoots(); container.remove(); document.body.innerHTML = ""; vi.clearAllMocks(); @@ -602,13 +621,14 @@ describe("Secrets page layout", () => { }); await flushReact(); + await waitForReact(() => document.body.textContent?.includes("View in Usage") ?? false); const viewUsageButton = Array.from(document.body.querySelectorAll("button")).find( (button) => button.textContent?.includes("View in Usage"), ) as HTMLButtonElement | undefined; await act(async () => { viewUsageButton?.click(); }); - await flushReact(); + await waitForReact(() => mockSecretsApi.usage.mock.calls.length > 0); expect(mockSecretsApi.usage).toHaveBeenCalledWith("secret-openai"); expect(document.body.textContent).toContain("CodexCoder"); @@ -619,6 +639,112 @@ describe("Secrets page layout", () => { }); }); + it("opens the secret detail sheet from a ?secret= deep link", async () => { + mockSecretsApi.list.mockResolvedValue([makeCompanySecret()]); + mockSecretsApi.usage.mockResolvedValue({ secretId: "secret-openai", bindings: [] }); + mockSecretsApi.accessEvents.mockResolvedValue([]); + + const root = createRoot(container); + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + + await act(async () => { + root.render( + + + + + , + ); + }); + await flushReact(); + await flushReact(); + + // The detail sheet is open without any clicks: the deep link drives selection. + const copyLinkButton = Array.from(document.body.querySelectorAll("button")).find( + (button) => button.textContent?.includes("Copy link"), + ); + expect(copyLinkButton).not.toBeUndefined(); + const sheet = copyLinkButton?.closest("[role='dialog']"); + expect(sheet?.textContent).toContain("OPENAI_API_KEY"); + + await act(async () => { + root.unmount(); + }); + }); + + it("writes a new value through the provider for external reference secrets", async () => { + const externalSecret = makeCompanySecret({ + id: "secret-neon", + key: "neon_admin_api_key", + name: "paperclip-cloud/prod/provider/neon/admin-api-key", + provider: "aws_secrets_manager", + managedMode: "external_reference", + externalRef: "arn:aws:secretsmanager:us-east-1:123456789012:secret:paperclip-cloud/prod/provider/neon/admin-api-key", + providerConfigId: "vault-aws", + }); + mockSecretsApi.list.mockResolvedValue([externalSecret]); + mockSecretsApi.usage.mockResolvedValue({ secretId: "secret-neon", bindings: [] }); + mockSecretsApi.accessEvents.mockResolvedValue([]); + mockSecretsApi.rotate.mockResolvedValue({ ...externalSecret, latestVersion: 2 }); + + const root = createRoot(container); + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + + await act(async () => { + root.render( + + + + + , + ); + }); + await flushReact(); + await flushReact(); + + // AWS supports write-through, so the primary action is a value update. + const updateValueButton = Array.from(document.body.querySelectorAll("button")).find( + (button) => button.textContent?.trim() === "Update value", + ) as HTMLButtonElement | undefined; + expect(updateValueButton).not.toBeUndefined(); + await act(async () => { + updateValueButton?.click(); + }); + await flushReact(); + + // Both modes are offered; "Write new value" is the default. + expect(document.body.textContent).toContain("Write new value"); + expect(document.body.textContent).toContain("Change reference"); + + const valueTextarea = document.getElementById("rotate-value") as HTMLTextAreaElement | null; + expect(valueTextarea).not.toBeNull(); + setTextareaValue(valueTextarea!, "rotated-neon-admin-key"); + await flushReact(); + + const dialog = valueTextarea!.closest("[role='dialog']") as HTMLElement; + const submitButton = Array.from(dialog.querySelectorAll("button")).find( + (button) => button.textContent?.trim() === "Update value", + ) as HTMLButtonElement | undefined; + expect(submitButton?.disabled).toBe(false); + await act(async () => { + submitButton?.click(); + }); + await flushReact(); + + expect(mockSecretsApi.rotate).toHaveBeenCalledWith("secret-neon", { + value: "rotated-neon-admin-key", + providerConfigId: "vault-aws", + }); + + await act(async () => { + root.unmount(); + }); + }); + it("merges company secrets and each-user definitions into the Secrets list", async () => { mockSecretsApi.list.mockResolvedValue([makeCompanySecret()]); mockSecretsApi.listUserSecretDefinitions.mockResolvedValue([makeUserSecretDefinition()]); @@ -822,7 +948,7 @@ describe("Secrets page layout", () => { await act(async () => { definitionRow?.click(); }); - await flushReact(); + await waitForReact(() => document.body.textContent?.includes("Details") ?? false); expect(document.body.textContent).toContain("Personal GitHub token"); expect(document.body.textContent).toContain("Details"); @@ -1350,8 +1476,7 @@ describe("Secrets page layout", () => { await act(async () => { companyRow?.click(); }); - await flushReact(); - await flushReact(); + await waitForReact(() => document.body.textContent?.includes("Reviewer") ?? false); // Existing access is listed right in the Details tab. expect(document.body.textContent).toContain("Agent access"); @@ -1506,6 +1631,7 @@ describe("Secrets folder view (PAP-14698)", () => { }); afterEach(() => { + unmountActiveRoots(); container.remove(); document.body.innerHTML = ""; vi.clearAllMocks(); diff --git a/ui/src/pages/Secrets.tsx b/ui/src/pages/Secrets.tsx index 112301986d0..14167e3c5f3 100644 --- a/ui/src/pages/Secrets.tsx +++ b/ui/src/pages/Secrets.tsx @@ -127,6 +127,9 @@ import { import type { MyUserSecretEntry } from "../api/secrets"; type CreateMode = "managed" | "external"; +// "value" writes a new secret value (for external references: through to the +// provider); "reference" re-points an external reference without writing. +type RotateMode = "value" | "reference"; type SecretValueProvider = "company" | "user"; type ProvidedByFilter = "all" | SecretValueProvider; type SecretsTab = "secrets" | "my-secrets" | "vaults"; @@ -341,9 +344,12 @@ function modeLabel(managedMode: SecretManagedMode) { return managedMode === "paperclip_managed" ? "Paperclip-managed" : "Linked external"; } -function modeDescription(managedMode: SecretManagedMode) { - return managedMode === "paperclip_managed" - ? "Paperclip owns create and rotation writes for this provider secret." +function modeDescription(managedMode: SecretManagedMode, canWriteExternalValue = false) { + if (managedMode === "paperclip_managed") { + return "Paperclip owns create and rotation writes for this provider secret."; + } + return canWriteExternalValue + ? "Paperclip resolves this provider reference and can write new values to it via Update value." : "Paperclip resolves this provider reference but does not rotate the provider value."; } @@ -650,8 +656,25 @@ export function Secrets() { const [statusFilter, setStatusFilter] = useState("active"); const [providerFilter, setProviderFilter] = useState("all"); const [providedByFilter, setProvidedByFilter] = useState("all"); - const [selectedSecretId, setSelectedSecretId] = useState(null); - const [selectedDefinitionId, setSelectedDefinitionId] = useState(null); + const [searchParams, setSearchParams] = useSearchParams(); + // The detail sheet is deep-linkable: `?secret=` / + // `?definition=` are the source of truth for the current selection, so + // every open secret has a shareable URL and Back closes the sheet. + const selectedSecretId = searchParams.get("secret"); + const selectedDefinitionId = searchParams.get("definition"); + const setDetailSelection = useCallback( + (secretId: string | null, definitionId: string | null = null) => { + setSearchParams((prev) => { + const next = new URLSearchParams(prev); + if (secretId) next.set("secret", secretId); + else next.delete("secret"); + if (definitionId) next.set("definition", definitionId); + else next.delete("definition"); + return next; + }); + }, + [setSearchParams], + ); const [usageDialogSecretId, setUsageDialogSecretId] = useState(null); const [createOpen, setCreateOpen] = useState(false); const [importOpen, setImportOpen] = useState(false); @@ -674,6 +697,7 @@ export function Secrets() { }); const [createError, setCreateError] = useState(null); const [rotateOpen, setRotateOpen] = useState(false); + const [rotateMode, setRotateMode] = useState("value"); const [rotateValue, setRotateValue] = useState(""); const [rotateExternalRef, setRotateExternalRef] = useState(""); const [rotateProviderConfigId, setRotateProviderConfigId] = useState(""); @@ -868,7 +892,6 @@ export function Secrets() { // Folders are derived purely from slash-delimited secret names; there is no // server-side folder record. `?path=` holds the normalized current folder // and is only meaningful on the main Secrets tab (inert on the others). - const [searchParams, setSearchParams] = useSearchParams(); const pathParam = normalizeSecretPath(searchParams.get("path") ?? ""); const folderPath = activeTab === "secrets" ? pathParam : ""; const searching = search.trim().length > 0; @@ -1094,12 +1117,10 @@ export function Secrets() { }); setCreateError(null); if (result.kind === "company") { - setSelectedSecretId(result.item.id); - setSelectedDefinitionId(null); + setDetailSelection(result.item.id); invalidateAll([result.item.id]); } else { - setSelectedDefinitionId(result.item.id); - setSelectedSecretId(null); + setDetailSelection(null, result.item.id); invalidateAll([result.item.id]); } }, @@ -1111,7 +1132,7 @@ export function Secrets() { const rotateMutation = useMutation({ mutationFn: () => { if (!selectedSecret) throw new Error("Select a secret first"); - if (selectedSecret.managedMode === "external_reference") { + if (selectedSecret.managedMode === "external_reference" && rotateMode === "reference") { return secretsApi.rotate(selectedSecret.id, { externalRef: rotateExternalRef.trim() || selectedSecret.externalRef || undefined, providerConfigId: rotateProviderConfigId || null, @@ -1183,7 +1204,7 @@ export function Secrets() { onSuccess: (_response, id) => { pushToast({ title: "Secret deleted", tone: "info" }); setDeleteConfirm(null); - if (selectedSecretId === id) setSelectedSecretId(null); + if (selectedSecretId === id) setDetailSelection(null); invalidateAll([id]); }, onError: (error) => { @@ -1201,7 +1222,7 @@ export function Secrets() { onSuccess: (_response, definition) => { pushToast({ title: "User-provided secret removed", body: definition.name, tone: "info" }); setDefinitionDeleteConfirm(null); - if (selectedDefinitionId === definition.id) setSelectedDefinitionId(null); + if (selectedDefinitionId === definition.id) setDetailSelection(null); invalidateAll([definition.id]); }, onError: (error) => { @@ -1409,19 +1430,36 @@ export function Secrets() { function openCompanySecret(secret: CompanySecret) { setSecretDetailTab("details"); - setSelectedSecretId(secret.id); - setSelectedDefinitionId(null); + setDetailSelection(secret.id); } function openUserDefinition(definition: UserSecretDefinition) { setSecretDetailTab("details"); - setSelectedDefinitionId(definition.id); - setSelectedSecretId(null); + setDetailSelection(null, definition.id); + } + + function secretSupportsExternalValueWrite(secret: CompanySecret) { + return ( + secret.managedMode === "external_reference" && + Boolean(secret.externalRef) && + Boolean(providers.find((provider) => provider.id === secret.provider)?.supportsExternalValueWrites) + ); + } + + function rotateActionLabel(secret: CompanySecret) { + return secret.managedMode === "external_reference" && !secretSupportsExternalValueWrite(secret) + ? "Update reference" + : "Update value"; } function openRotateSecret(secret: CompanySecret) { openCompanySecret(secret); setRotateOpen(true); + setRotateMode( + secret.managedMode === "external_reference" && !secretSupportsExternalValueWrite(secret) + ? "reference" + : "value", + ); setRotateValue(""); setRotateExternalRef(""); setRotateProviderConfigId( @@ -1430,6 +1468,18 @@ export function Secrets() { setRotateError(null); } + function copyDetailLink() { + void copyTextToClipboard(window.location.href) + .then(() => pushToast({ title: "Link copied", body: "Deep link to this secret", tone: "success" })) + .catch((error) => + pushToast({ + title: "Copy failed", + body: error instanceof Error ? error.message : "Unable to copy link", + tone: "error", + }), + ); + } + function copySecretKey(key: string) { void copyTextToClipboard(key) .then(() => pushToast({ title: "Secret key copied", body: key, tone: "success" })) @@ -1472,7 +1522,7 @@ export function Secrets() { openRotateSecret(row.secret)}> - {row.secret.managedMode === "external_reference" ? "Update reference" : "Update value"} + {rotateActionLabel(row.secret)} { - if (!open) { - setSelectedSecretId(null); - setSelectedDefinitionId(null); - } + if (!open && (selectedSecret || selectedDefinition)) setDetailSelection(null); }} > @@ -2188,7 +2235,10 @@ export function Secrets() { onClick={() => openRotateSecret(selectedSecret)} > - {selectedSecret.managedMode === "external_reference" ? "Update reference" : "Update value"} + {rotateActionLabel(selectedSecret)} + + @@ -2964,14 +3014,26 @@ export function Secrets() { - {selectedSecret?.managedMode === "external_reference" ? "Update external reference" : "Update secret value"} + {selectedSecret?.managedMode === "external_reference" && rotateMode === "reference" + ? "Update external reference" + : "Update secret value"} - {selectedSecret?.managedMode === "external_reference" - ? "Creates a new Paperclip metadata version that points at an existing provider secret. Paperclip does not write a new provider value." - : "Creates a new provider-backed version. Consumers pinned to latest pick up the new value on the next run."} + {selectedSecret?.managedMode !== "external_reference" + ? "Creates a new provider-backed version. Consumers pinned to latest pick up the new value on the next run." + : rotateMode === "reference" + ? "Creates a new Paperclip metadata version that points at an existing provider secret. Paperclip does not write a new provider value." + : "Writes a new version of the referenced provider secret. The new value becomes current for every consumer of that secret, in and outside Paperclip."} + {selectedSecret && secretSupportsExternalValueWrite(selectedSecret) ? ( + setRotateMode(value as RotateMode)}> + + Write new value + Change reference + + + ) : null}
+ {selectedSecret?.managedMode === "external_reference" ? ( +

+ Written to {selectedSecret.externalRef} in the provider. +

+ ) : null}
)} {rotateError ?

{rotateError}

: null} @@ -3040,13 +3107,15 @@ export function Secrets() { disabled={ rotateMutation.isPending || Boolean(rotateProviderBlockReason) || - (selectedSecret?.managedMode === "external_reference" + (selectedSecret?.managedMode === "external_reference" && rotateMode === "reference" ? !rotateExternalRef.trim() && !selectedSecret?.externalRef : !rotateValue) } > {rotateMutation.isPending ? : null} - {selectedSecret?.managedMode === "external_reference" ? "Update reference" : "Update value"} + {selectedSecret?.managedMode === "external_reference" && rotateMode === "reference" + ? "Update reference" + : "Update value"} @@ -4546,7 +4615,14 @@ function SecretDetailsTab({ {formatRelative(secret.lastRotatedAt)} {formatRelative(secret.lastResolvedAt)}
- {modeDescription(secret.managedMode)} Paperclip never re-displays stored values. + {modeDescription( + secret.managedMode, + Boolean( + secret.externalRef && + providers.find((provider) => provider.id === secret.provider)?.supportsExternalValueWrites, + ), + )}{" "} + Paperclip never re-displays stored values.
);