diff --git a/server/src/__tests__/environment-routes.test.ts b/server/src/__tests__/environment-routes.test.ts index 5a8cc7a5fb..06c04d2d94 100644 --- a/server/src/__tests__/environment-routes.test.ts +++ b/server/src/__tests__/environment-routes.test.ts @@ -66,6 +66,7 @@ const mockSecretService = vi.hoisted(() => ({ syncEnvBindingsForTarget: vi.fn(), syncSecretRefsForTarget: vi.fn(), replaceSecretRefsForInstanceTarget: vi.fn(), + describeSecretRefs: vi.fn(), remove: vi.fn(), })); const mockValidatePluginEnvironmentDriverConfig = vi.hoisted(() => vi.fn()); @@ -269,6 +270,8 @@ describe("environment routes", () => { mockSecretService.syncEnvBindingsForTarget.mockReset(); mockSecretService.syncSecretRefsForTarget.mockReset(); mockSecretService.replaceSecretRefsForInstanceTarget.mockReset(); + mockSecretService.describeSecretRefs.mockReset(); + mockSecretService.describeSecretRefs.mockResolvedValue([]); mockSecretService.remove.mockReset(); mockSecretService.create.mockResolvedValue({ id: "11111111-1111-1111-1111-111111111111", @@ -1224,6 +1227,76 @@ describe("environment routes", () => { expect(mockEnvironmentService.removeIfDeletable).not.toHaveBeenCalled(); }); + it("describes an environment's secret refs with owner metadata", async () => { + const secretId = "22222222-2222-2222-2222-222222222222"; + mockEnvironmentService.getById.mockResolvedValue({ + ...createEnvironment(), + id: "env-sandbox", + name: "Daytona", + driver: "sandbox" as const, + config: { + provider: "secure-plugin", + template: "base", + apiKey: secretId, + timeoutMs: 450000, + reuseLease: true, + }, + }); + mockSecretService.describeSecretRefs.mockResolvedValue([ + { + configPath: "apiKey", + secretId, + name: "DAYTONA_API_KEY", + status: "active", + companyId: "company-2", + companyName: "Other Team", + }, + ]); + const app = createApp({ + type: "board", + userId: "admin-1", + source: "local_implicit", + }); + + const res = await request(app).get("/api/environments/env-sandbox/secret-refs"); + + expect(res.status).toBe(200); + expect(res.body.refs).toEqual([ + { + configPath: "apiKey", + secretId, + name: "DAYTONA_API_KEY", + status: "active", + companyId: "company-2", + companyName: "Other Team", + }, + ]); + expect(mockSecretService.describeSecretRefs).toHaveBeenCalledWith([ + { secretId, configPath: "apiKey", versionSelector: "latest" }, + ]); + }); + + it("denies secret-ref descriptors to agents without instance environment access", async () => { + mockAgentService.getById.mockResolvedValue({ + id: "agent-1", + companyId: "company-1", + role: "engineer", + permissions: { canCreateAgents: false }, + }); + mockAccessService.hasPermission.mockResolvedValue(false); + const app = createApp({ + type: "agent", + agentId: "agent-1", + companyId: "company-1", + source: "agent_key", + }); + + const res = await request(app).get("/api/environments/env-1/secret-refs"); + + expect(res.status).toBe(403); + expect(mockSecretService.describeSecretRefs).not.toHaveBeenCalled(); + }); + it("clears environment selections and secret bindings across all companies when deleting an environment", async () => { const environment = { ...createEnvironment(), diff --git a/server/src/__tests__/secrets-service.test.ts b/server/src/__tests__/secrets-service.test.ts index a3855f9b72..bd30d873c2 100644 --- a/server/src/__tests__/secrets-service.test.ts +++ b/server/src/__tests__/secrets-service.test.ts @@ -286,6 +286,47 @@ describeEmbeddedPostgres("secretService", () => { expect(rows[0]?.companyId).toBe(companyA); }); + it("describeSecretRefs names secrets across companies and omits unknown ids", async () => { + const companyA = await seedCompany("Alpha"); + const companyB = await seedCompany("Beta"); + const svc = secretService(db); + const secretA = await svc.create(companyA, { + name: "PROVIDER_KEY_A", + provider: "local_encrypted", + value: "a", + }); + const secretB = await svc.create(companyB, { + name: "PROVIDER_KEY_B", + provider: "local_encrypted", + value: "b", + }); + + const described = await svc.describeSecretRefs([ + { secretId: secretA.id, configPath: "apiKey" }, + { secretId: secretB.id, configPath: "privateKeySecretRef" }, + { secretId: randomUUID(), configPath: "token" }, + ]); + + expect(described).toEqual([ + { + configPath: "apiKey", + secretId: secretA.id, + name: "PROVIDER_KEY_A", + status: "active", + companyId: companyA, + companyName: "Alpha", + }, + { + configPath: "privateKeySecretRef", + secretId: secretB.id, + name: "PROVIDER_KEY_B", + status: "active", + companyId: companyB, + companyName: "Beta", + }, + ]); + }); + it("prevents duplicate bindings for a target config path", async () => { const companyId = await seedCompany(); const svc = secretService(db); diff --git a/server/src/routes/environments.ts b/server/src/routes/environments.ts index 1ca4259506..087b60c532 100644 --- a/server/src/routes/environments.ts +++ b/server/src/routes/environments.ts @@ -932,6 +932,22 @@ export function environmentRoutes( res.json(presentEnvironmentForRead(req, environment)); }); + router.get("/environments/:id/secret-refs", async (req, res) => { + assertCanAccessInstanceEnvironments(req); + const environment = await svc.getById(req.params.id as string); + if (!environment) { + res.status(404).json({ error: "Environment not found" }); + return; + } + // Metadata only (name / status / owning company) — never secret values. + // Environments are instance-scoped while secrets are company-scoped, so + // the editor needs this to render refs whose secret a given company's + // picker cannot list. Gated by the same instance-level access check as + // environment editing. + const refs = await collectEnvironmentSecretRefs({ db, environment }); + res.json({ refs: await secrets.describeSecretRefs(refs) }); + }); + router.get("/environments/:id/leases", async (req, res) => { assertCanReadInstanceEnvironments(req); const environment = await svc.getById(req.params.id as string); diff --git a/server/src/routes/openapi.ts b/server/src/routes/openapi.ts index 43fd6a4fe4..ebfd82fd5e 100644 --- a/server/src/routes/openapi.ts +++ b/server/src/routes/openapi.ts @@ -4576,6 +4576,15 @@ registry.registerPath({ responses: { 200: r.ok(), 401: r.unauthorized, 403: r.forbidden, 404: r.notFound }, }); +registry.registerPath({ + method: "get", + path: "/api/environments/{id}/secret-refs", + tags: ["environments"], + summary: "Describe an environment's config secret refs (name, status, owning company — never values)", + request: { params: z.object({ id: z.string() }) }, + responses: { 200: r.ok(), 401: r.unauthorized, 403: r.forbidden, 404: r.notFound }, +}); + registry.registerPath({ method: "get", path: "/api/environments/{id}/leases", diff --git a/server/src/services/secrets.ts b/server/src/services/secrets.ts index db97905975..aa0b7124ff 100644 --- a/server/src/services/secrets.ts +++ b/server/src/services/secrets.ts @@ -3,6 +3,7 @@ import { and, desc, eq, inArray, like, ne, notInArray, notLike, or, sql } from " import type { Db } from "@paperclipai/db"; import { agents, + companies, companySecretBindings, companySecretProviderConfigs, companySecrets, @@ -4120,6 +4121,51 @@ export function secretService(db: Db) { return normalizedRefs; }, + /** + * Describe secret refs (id + config path) with the referenced secret's + * name, status, and owning company. Environments are instance-scoped + * while secrets are company-scoped, so an environment can legitimately + * reference a secret a given company's picker cannot list; this gives + * instance-level readers enough metadata to present such refs honestly. + * Returns names across companies — callers must sit behind an + * instance-level authorization gate. Never returns secret values. + */ + describeSecretRefs: async ( + refs: Array<{ secretId: string; configPath: string }>, + ): Promise> => { + if (refs.length === 0) return []; + const secretIds = [...new Set(refs.map((ref) => ref.secretId))]; + const secretRows = await db + .select() + .from(companySecrets) + .where(inArray(companySecrets.id, secretIds)); + const secretsById = new Map(secretRows.map((row) => [row.id, row])); + const companyIds = [...new Set(secretRows.map((row) => row.companyId))]; + const companyRows = companyIds.length > 0 + ? await db.select().from(companies).where(inArray(companies.id, companyIds)) + : []; + const companyNamesById = new Map(companyRows.map((row) => [row.id, row.name])); + return refs.flatMap((ref) => { + const secret = secretsById.get(ref.secretId); + if (!secret) return []; + return [{ + configPath: ref.configPath, + secretId: secret.id, + name: secret.name, + status: secret.status, + companyId: secret.companyId, + companyName: companyNamesById.get(secret.companyId) ?? null, + }]; + }); + }, + listBindingCompanyIdsForTarget: async ( target: { targetType: SecretBindingTargetType; targetId: string }, ): Promise => diff --git a/ui/src/api/environments.ts b/ui/src/api/environments.ts index 030def1198..e3ebe8a370 100644 --- a/ui/src/api/environments.ts +++ b/ui/src/api/environments.ts @@ -58,11 +58,22 @@ function customImageCompanyQuery(companyId: string): string { return `companyId=${encodeURIComponent(companyId)}`; } +export interface EnvironmentSecretRefDescriptor { + configPath: string; + secretId: string; + name: string; + status: string; + companyId: string; + companyName: string | null; +} + export const environmentsApi = { list: (companyId: string) => api.get(`/companies/${companyId}/environments`), capabilities: (companyId: string) => api.get(`/companies/${companyId}/environments/capabilities`), lease: (leaseId: string) => api.get(`/environment-leases/${leaseId}`), + secretRefs: (environmentId: string) => + api.get<{ refs: EnvironmentSecretRefDescriptor[] }>(`/environments/${environmentId}/secret-refs`), create: (companyId: string, body: { name: string; description?: string | null; diff --git a/ui/src/components/SecretBindingPicker.test.tsx b/ui/src/components/SecretBindingPicker.test.tsx new file mode 100644 index 0000000000..476ab5b5d3 --- /dev/null +++ b/ui/src/components/SecretBindingPicker.test.tsx @@ -0,0 +1,144 @@ +// @vitest-environment jsdom + +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + SecretBindingPicker, + SecretRefHintsContext, + type SecretRefHintsContextValue, +} from "./SecretBindingPicker"; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true; + +const mockSecretsApi = vi.hoisted(() => ({ + list: vi.fn(), + create: vi.fn(), +})); + +vi.mock("../api/secrets", () => ({ + secretsApi: mockSecretsApi, +})); + +vi.mock("../context/CompanyContext", () => ({ + useCompany: () => ({ selectedCompanyId: "company-1" }), +})); + +describe("SecretBindingPicker", () => { + let container: HTMLDivElement; + let root: Root | null = null; + let queryClient: QueryClient; + + beforeEach(() => { + container = document.createElement("div"); + document.body.appendChild(container); + queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + mockSecretsApi.list.mockReset(); + mockSecretsApi.list.mockResolvedValue([]); + }); + + afterEach(() => { + act(() => { + root?.unmount(); + }); + root = null; + container.remove(); + queryClient.clear(); + }); + + async function render(context: SecretRefHintsContextValue | undefined) { + await act(async () => { + root = createRoot(container); + root.render( + + + {}} + /> + + , + ); + }); + // Let the secrets query settle so selectedMissing is based on real data. + await act(async () => { + await Promise.resolve(); + }); + } + + function readyContext(status: string): SecretRefHintsContextValue { + return { + status: "ready", + hints: { + "22222222-2222-2222-2222-222222222222": { + name: "DAYTONA_API_KEY", + status, + companyId: "company-2", + companyName: "Other Team", + }, + }, + }; + } + + it("names an active cross-company secret and its owner instead of calling it missing", async () => { + await render(readyContext("active")); + + expect(container.textContent).toContain("DAYTONA_API_KEY — Other Team"); + expect(container.textContent).toContain("Owned by the Other Team company"); + expect(container.textContent).not.toContain("Missing secret"); + expect(container.querySelector("select")?.className).not.toContain("border-destructive"); + }); + + it("reports a deleted hinted secret as deleted", async () => { + await render(readyContext("deleted")); + + expect(container.textContent).toContain("DAYTONA_API_KEY — Other Team"); + expect(container.textContent).toContain("was deleted"); + expect(container.querySelector("select")?.className).toContain("border-destructive"); + }); + + it("does not present a disabled cross-company secret as working", async () => { + await render(readyContext("disabled")); + + expect(container.textContent).toContain("DAYTONA_API_KEY — Other Team"); + expect(container.textContent).toContain("This secret is disabled"); + expect(container.textContent).not.toContain("keeps working"); + expect(container.querySelector("select")?.className).toContain("border-destructive"); + }); + + it("stays neutral while descriptors are loading", async () => { + await render({ status: "loading", hints: {} }); + + expect(container.textContent).toContain("Checking this secret reference"); + expect(container.textContent).not.toContain("Missing secret"); + expect(container.querySelector("select")?.className).not.toContain("border-destructive"); + }); + + it("stays neutral when the descriptor lookup failed", async () => { + await render({ status: "error", hints: {} }); + + expect(container.textContent).toContain("Could not load this secret reference"); + expect(container.textContent).not.toContain("Missing secret"); + expect(container.querySelector("select")?.className).not.toContain("border-destructive"); + }); + + it("treats an unknown id as missing once descriptors are ready", async () => { + await render({ status: "ready", hints: {} }); + + expect(container.textContent).toContain("Missing secret"); + expect(container.textContent).toContain("no longer available"); + expect(container.querySelector("select")?.className).toContain("border-destructive"); + }); + + it("keeps the generic missing-secret treatment when no hint context exists", async () => { + await render(undefined); + + expect(container.textContent).toContain("Missing secret"); + expect(container.textContent).toContain("no longer available"); + expect(container.querySelector("select")?.className).toContain("border-destructive"); + }); +}); diff --git a/ui/src/components/SecretBindingPicker.tsx b/ui/src/components/SecretBindingPicker.tsx index 1bf33d19d3..a25891f0a4 100644 --- a/ui/src/components/SecretBindingPicker.tsx +++ b/ui/src/components/SecretBindingPicker.tsx @@ -1,4 +1,4 @@ -import { useMemo, useState } from "react"; +import { createContext, useContext, useMemo, useState } from "react"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { AlertCircle, KeyRound, Loader2, Plus, X } from "lucide-react"; import type { CompanySecret, SecretVersionSelector } from "@paperclipai/shared"; @@ -16,6 +16,32 @@ export interface SecretBindingValue { version?: SecretVersionSelector; } +/** + * Metadata for bound secrets the current company's list cannot show — e.g. + * an instance-scoped environment referencing a secret owned by another + * company. Keyed by secret id. Editors that can read instance-level + * secret-ref descriptors provide it; everywhere else the context is absent + * and the picker falls back to its generic missing-secret treatment. + */ +export interface SecretRefHint { + name: string; + status: string; + companyId: string; + companyName: string | null; +} + +/** + * `status` reports the descriptor request itself, so the picker never claims + * a secret is missing while the lookup is still loading or has failed — + * only a `ready` map is authoritative about unknown ids. + */ +export interface SecretRefHintsContextValue { + status: "loading" | "error" | "ready"; + hints: Record; +} + +export const SecretRefHintsContext = createContext(undefined); + interface SecretBindingPickerProps { value: SecretBindingValue | null; onChange: (next: SecretBindingValue | null) => void; @@ -96,6 +122,14 @@ export function SecretBindingPicker({ }, [secretsQuery.data, value]); const selectedMissing = Boolean(value && !selectedSecret); + const hintsContext = useContext(SecretRefHintsContext); + const missingHint = selectedMissing && value ? hintsContext?.hints[value.secretId] : undefined; + // Only an active cross-company secret is healthy: runtime resolution + // rejects disabled/archived/deleted secrets, so those must not be + // presented as working bindings. + const crossCompanyHint = missingHint && missingHint.status === "active" ? missingHint : undefined; + const hintsPending = selectedMissing && !missingHint && hintsContext !== undefined && hintsContext.status !== "ready"; + const calmMissing = Boolean(crossCompanyHint) || hintsPending; const createMutation = useMutation({ mutationFn: () => @@ -146,7 +180,7 @@ export function SecretBindingPicker({