diff --git a/docs/api/secrets.md b/docs/api/secrets.md index 5caf15272eb..7f92fcf4b2d 100644 --- a/docs/api/secrets.md +++ b/docs/api/secrets.md @@ -38,6 +38,16 @@ GET /api/agents/me/secrets IDs, binding IDs, or config paths. An `env.*` binding implies read access through this API; an `access.*` binding grants API access without environment injection. +`key` is the key the grant is *projected* under, which is not always the managed +secret's own key. An `env.*` binding is listed under the managed secret key it is +injected from, while an `access.` binding is listed under ``, +normalized the same way managed secret keys are (lower-cased). So one managed +secret can back several grants — for example `access.EVALS_OPENAI_API_KEY` and +`access.RUNNERD_OPENAI_API_KEY` over a single shared key are listed as two +separate `api` entries, each fetched by its own alias. A secret that has both an +`env.*` binding and an `access.*` alias equal to its key is listed once, as +`both`. + Fetch a value only when it is needed. The request has no body and the response uses `Cache-Control: no-store`: diff --git a/server/src/__tests__/agent-secrets-routes.test.ts b/server/src/__tests__/agent-secrets-routes.test.ts index da0543c26c2..03e5ca7ac8b 100644 --- a/server/src/__tests__/agent-secrets-routes.test.ts +++ b/server/src/__tests__/agent-secrets-routes.test.ts @@ -226,6 +226,92 @@ describeEmbeddedPostgres("agent secret routes", () => { ])); }); + it("projects API grants under their access alias and keeps distinct aliases for one secret", async () => { + const fixture = await seedAgentRun(); + const svc = secretService(db); + // The managed key deliberately does not match either alias: an API-only grant + // must be addressable by the alias the operator approved, not by the reused + // secret's own key. + const sharedSecret = await svc.create(fixture.companyId, { + key: "OPENAI_API_KEY_DOTTA_PAPERCLLIPING", + name: "Shared OpenAI key", + provider: "local_encrypted", + value: "shared-openai-value", + }); + const pairedSecret = await svc.create(fixture.companyId, { + key: "PAIRED_KEY", + name: "Injected and fetchable", + provider: "local_encrypted", + value: "paired-secret-value", + }); + const evalsBinding = await svc.createBinding({ + companyId: fixture.companyId, + secretId: sharedSecret.id, + targetType: "agent", + targetId: fixture.agentId, + configPath: "access.EVALS_OPENAI_API_KEY", + projectionClass: "class_2_runtime_only", + }); + const runnerdBinding = await svc.createBinding({ + companyId: fixture.companyId, + secretId: sharedSecret.id, + targetType: "agent", + targetId: fixture.agentId, + configPath: "access.runnerd_openai_api_key", + }); + // env + access that project the same key stay one entry, delivered "both". + await svc.createBinding({ + companyId: fixture.companyId, + secretId: pairedSecret.id, + targetType: "agent", + targetId: fixture.agentId, + configPath: "env.PAIRED_KEY", + }); + await svc.createBinding({ + companyId: fixture.companyId, + secretId: pairedSecret.id, + targetType: "agent", + targetId: fixture.agentId, + configPath: "access.PAIRED_KEY", + }); + + const list = await request(createApp(fixture)).get("/api/agents/me/secrets"); + expect(list.status).toBe(200); + expect(list.body.secrets).toEqual([ + expect.objectContaining({ + key: "evals_openai_api_key", + name: "Shared OpenAI key", + delivery: "api", + projectionClass: "class_2_runtime_only", + }), + expect.objectContaining({ key: "paired_key", delivery: "both" }), + expect.objectContaining({ key: "runnerd_openai_api_key", delivery: "api" }), + ]); + expect(list.body.secrets.map((secret: { key: string }) => secret.key)) + .not.toContain("openai_api_key_dotta_paperclliping"); + expect(JSON.stringify(list.body)).not.toContain("-value"); + + // Each alias resolves through its own binding, not a single collapsed one. + for (const alias of ["evals_openai_api_key", "runnerd_openai_api_key"]) { + const fetched = await request(createApp(fixture)).post(`/api/agents/me/secrets/${alias}/value`); + expect(fetched.status, alias).toBe(200); + expect(fetched.body, alias).toEqual({ key: alias, value: "shared-openai-value", version: 1 }); + } + const pairedFetched = await request(createApp(fixture)).post("/api/agents/me/secrets/paired_key/value"); + expect(pairedFetched.status).toBe(200); + expect(pairedFetched.body).toEqual({ key: "paired_key", value: "paired-secret-value", version: 1 }); + + // The reused managed key is not itself a grant, so it stays unaddressable. + const byManagedKey = await request(createApp(fixture)) + .post("/api/agents/me/secrets/openai_api_key_dotta_paperclliping/value"); + expect(byManagedKey.status).toBe(403); + + const events = await db.select().from(secretAccessEvents); + expect(events.filter((event) => event.secretId === sharedSecret.id).map((event) => event.configPath).sort()) + .toEqual([evalsBinding.configPath, runnerdBinding.configPath].sort()); + expect(events.every((event) => event.outcome === "success")).toBe(true); + }); + it("fails closed when run redaction registration cannot be persisted", async () => { const fixture = await seedAgentRun(); await expect(createRunSecretRedactionRegistry(db).register(fixture.companyId, randomUUID(), "must-not-return")) diff --git a/server/src/__tests__/secrets-routes.test.ts b/server/src/__tests__/secrets-routes.test.ts index aed40e872fb..ce9ea4c8e1c 100644 --- a/server/src/__tests__/secrets-routes.test.ts +++ b/server/src/__tests__/secrets-routes.test.ts @@ -665,6 +665,89 @@ describe("secret routes", () => { })); }); + describe("agent secret access", () => { + const agentActor = { + type: "agent", + agentId: "agent-1", + companyId: "company-1", + source: "agent_jwt", + runId: "22222222-2222-4222-8222-222222222222", + }; + const aliasEntries = [ + { + secretId: "33333333-3333-4333-8333-333333333333", + bindingId: "44444444-4444-4444-8444-444444444444", + configPath: "access.evals_openai_api_key", + key: "evals_openai_api_key", + name: "Vendor OpenAI", + description: null, + delivery: "api", + projectionClass: "unclassified", + latestVersion: 2, + versionSelector: "latest", + resolvedVersion: 2, + }, + { + secretId: "33333333-3333-4333-8333-333333333333", + bindingId: "55555555-5555-4555-8555-555555555555", + configPath: "access.legacy_alias", + key: "legacy_alias", + name: "Vendor OpenAI", + description: null, + delivery: "api", + projectionClass: "unclassified", + latestVersion: 2, + versionSelector: "latest", + resolvedVersion: 2, + }, + ]; + + it("lists alias-projected metadata without binding internals or secret bytes", async () => { + mockSecretService.listAgentSecretAccess.mockResolvedValue(aliasEntries); + + const res = await request(createApp(agentActor)).get("/api/agents/me/secrets"); + + expect(res.status).toBe(200); + expect(res.body.secrets.map((entry: { key: string }) => entry.key)) + .toEqual(["evals_openai_api_key", "legacy_alias"]); + for (const entry of res.body.secrets) { + expect(entry).not.toHaveProperty("secretId"); + expect(entry).not.toHaveProperty("bindingId"); + expect(entry).not.toHaveProperty("configPath"); + expect(entry).not.toHaveProperty("value"); + } + }); + + it("resolves the exact projected binding when two aliases share one secret", async () => { + mockSecretService.listAgentSecretAccess.mockResolvedValue(aliasEntries); + mockSecretService.resolveSecretValueForAgentAccess.mockResolvedValue({ value: "fixture-value", version: 2 }); + + const res = await request(createApp(agentActor)).post("/api/agents/me/secrets/legacy_alias/value"); + + expect(res.status).toBe(200); + expect(res.body).toEqual({ key: "legacy_alias", value: "fixture-value", version: 2 }); + expect(mockSecretService.resolveSecretValueForAgentAccess).toHaveBeenCalledWith( + "company-1", + "33333333-3333-4333-8333-333333333333", + "latest", + expect.objectContaining({ + configPath: "access.legacy_alias", + bindingId: "55555555-5555-4555-8555-555555555555", + }), + ); + }); + + it("rejects a value read for the underlying managed secret key when only an alias is granted", async () => { + mockSecretService.listAgentSecretAccess.mockResolvedValue(aliasEntries); + + const res = await request(createApp(agentActor)) + .post("/api/agents/me/secrets/openai_api_key_dotta_paperclliping/value"); + + expect(res.status).toBe(403); + expect(mockSecretService.resolveSecretValueForAgentAccess).not.toHaveBeenCalled(); + }); + }); + it("rejects remote import preview for non-board actors", async () => { const res = await request(createApp({ type: "agent", diff --git a/server/src/__tests__/secrets-service.test.ts b/server/src/__tests__/secrets-service.test.ts index bd30d873c25..6dcfadeba66 100644 --- a/server/src/__tests__/secrets-service.test.ts +++ b/server/src/__tests__/secrets-service.test.ts @@ -386,6 +386,153 @@ describeEmbeddedPostgres("secretService", () => { })).rejects.toThrow(/invalid agent secret access alias/i); }); + it("lists API-only access bindings under their alias, not the managed secret key", async () => { + const companyId = await seedCompany(); + const svc = secretService(db); + const { agentId, heartbeatRunId } = await seedAgentRun(companyId); + // The managed key and the granted alias intentionally differ: the agent must + // see the alias it was granted, not the shared underlying secret key. + const secret = await svc.create(companyId, { + name: `vendor-openai-shared-${randomUUID()}`, + provider: "local_encrypted", + value: "runtime-secret", + }); + await svc.createBinding({ + companyId, + secretId: secret.id, + targetType: "agent", + targetId: agentId, + configPath: "access.evals_openai_api_key", + }); + + const listed = await svc.listAgentSecretAccess(companyId, { + agentId, + actorSource: "agent_jwt", + heartbeatRunId, + }); + + expect(secret.key).not.toBe("evals_openai_api_key"); + expect(listed).toHaveLength(1); + expect(listed[0]).toMatchObject({ + key: "evals_openai_api_key", + configPath: "access.evals_openai_api_key", + delivery: "api", + secretId: secret.id, + }); + // Metadata only: the listing must never carry secret bytes. + expect(JSON.stringify(listed)).not.toContain("runtime-secret"); + }); + + it("keeps distinct access aliases that reference the same secret readable", async () => { + const companyId = await seedCompany(); + const svc = secretService(db); + const { agentId, heartbeatRunId } = await seedAgentRun(companyId); + const secret = await svc.create(companyId, { + name: `shared-alias-${randomUUID()}`, + provider: "local_encrypted", + value: "runtime-secret", + }); + for (const alias of ["access.PRIMARY_ALIAS", "access.SECONDARY_ALIAS"]) { + await svc.createBinding({ + companyId, + secretId: secret.id, + targetType: "agent", + targetId: agentId, + configPath: alias, + }); + } + + const listed = await svc.listAgentSecretAccess(companyId, { + agentId, + actorSource: "agent_jwt", + heartbeatRunId, + }); + + // Aliases project through the same key normalization managed keys use. + expect(listed.map((entry) => entry.key)).toEqual(["primary_alias", "secondary_alias"]); + expect(listed.map((entry) => entry.configPath)).toEqual(["access.PRIMARY_ALIAS", "access.SECONDARY_ALIAS"]); + expect(new Set(listed.map((entry) => entry.bindingId)).size).toBe(2); + expect(listed.every((entry) => entry.delivery === "api")).toBe(true); + // Each projected entry must resolve through its own binding. + for (const entry of listed) { + await expect(svc.resolveSecretValueForAgentAccess(companyId, entry.secretId, entry.versionSelector, { + agentId, + configPath: entry.configPath, + bindingId: entry.bindingId, + actorSource: "agent_jwt", + heartbeatRunId, + registerForRedaction: () => {}, + })).resolves.toEqual({ value: "runtime-secret", version: 1 }); + } + }); + + it("collapses an env and access pair that project the same key into one both-delivery entry", async () => { + const companyId = await seedCompany(); + const svc = secretService(db); + const { agentId, heartbeatRunId } = await seedAgentRun(companyId); + const secret = await svc.create(companyId, { + // Underscores only: the alias below reuses this key and must stay a valid alias. + name: `dual_delivery_${randomUUID().replace(/-/g, "_")}`, + provider: "local_encrypted", + value: "runtime-secret", + }); + // env.* keeps projecting the managed secret key, so an alias equal to that key + // is the same projected key delivered two ways. + for (const configPath of [`access.${secret.key}`, "env.SOME_ENV_NAME"]) { + await svc.createBinding({ + companyId, + secretId: secret.id, + targetType: "agent", + targetId: agentId, + configPath, + }); + } + + const listed = await svc.listAgentSecretAccess(companyId, { + agentId, + actorSource: "agent_jwt", + heartbeatRunId, + }); + + expect(listed).toHaveLength(1); + expect(listed[0]).toMatchObject({ + key: secret.key, + delivery: "both", + configPath: `access.${secret.key}`, + }); + }); + + it("lists an env-only binding under the managed secret key", async () => { + const companyId = await seedCompany(); + const svc = secretService(db); + const { agentId, heartbeatRunId } = await seedAgentRun(companyId); + const secret = await svc.create(companyId, { + name: `env-only-${randomUUID()}`, + provider: "local_encrypted", + value: "runtime-secret", + }); + await svc.createBinding({ + companyId, + secretId: secret.id, + targetType: "agent", + targetId: agentId, + configPath: "env.SOME_ENV_NAME", + }); + + const listed = await svc.listAgentSecretAccess(companyId, { + agentId, + actorSource: "agent_jwt", + heartbeatRunId, + }); + + expect(listed).toHaveLength(1); + expect(listed[0]).toMatchObject({ + key: secret.key, + configPath: "env.SOME_ENV_NAME", + delivery: "env", + }); + }); + it("resolves env and access bindings through the run-bound agent resolver with dual audit", async () => { const companyId = await seedCompany(); const svc = secretService(db); diff --git a/server/src/services/secrets.ts b/server/src/services/secrets.ts index 20bec111a77..b3730ead650 100644 --- a/server/src/services/secrets.ts +++ b/server/src/services/secrets.ts @@ -1486,38 +1486,97 @@ export function secretService(db: Db) { inArray(companySecrets.id, [...new Set(bindings.map((binding) => binding.secretId))]), )); const secretsById = new Map(secrets.map((secret) => [secret.id, secret])); - const bindingsBySecret = new Map(); + + // Each binding projects its own metadata key: API-only bindings project the + // `access.` alias, env bindings keep projecting the managed secret key. + // Grouping by (secretId, projected key) keeps distinct aliases on one secret + // separate while still collapsing an env/API pair that projects the same key + // into a single `delivery: "both"` entry. + type ProjectedBinding = { key: string; isApi: boolean; binding: (typeof bindings)[number] }; + const projected: ProjectedBinding[] = []; for (const binding of bindings) { - const current = bindingsBySecret.get(binding.secretId) ?? []; - current.push(binding); - bindingsBySecret.set(binding.secretId, current); + const secret = secretsById.get(binding.secretId); + if (!secret) continue; + const isApi = binding.configPath.startsWith(AGENT_ACCESS_CONFIG_PATH_PREFIX); + if (!isApi) { + projected.push({ key: secret.key, isApi, binding }); + continue; + } + const alias = binding.configPath.slice(AGENT_ACCESS_CONFIG_PATH_PREFIX.length); + // Aliases are validated on write; skip legacy rows that could never be read back. + if (!ENV_KEY_RE.test(alias)) continue; + // Normalize the alias the same way managed secret keys are normalized so the + // projected key an agent addresses is spelled consistently, and so an alias + // that names its own secret merges with the env grant into `delivery: "both"` + // instead of splitting into two entries. + projected.push({ key: normalizeSecretKey(alias), isApi, binding }); } - return [...bindingsBySecret.entries()].flatMap(([secretId, secretBindings]) => { - const secret = secretsById.get(secretId); + const groups = new Map(); + for (const entry of projected) { + const groupKey = `${entry.binding.secretId}::${entry.key}`; + const current = groups.get(groupKey) ?? []; + current.push(entry); + groups.set(groupKey, current); + } + + // Prefer the API binding so the value route resolves against the alias path, + // then fall back to the oldest binding for a stable selection. + const bindingRank = (entry: ProjectedBinding) => [ + entry.isApi ? 0 : 1, + new Date(entry.binding.createdAt).getTime(), + entry.binding.id, + ] as const; + const compareBindings = (left: ProjectedBinding, right: ProjectedBinding) => { + const [leftKind, leftCreated, leftId] = bindingRank(left); + const [rightKind, rightCreated, rightId] = bindingRank(right); + if (leftKind !== rightKind) return leftKind - rightKind; + if (leftCreated !== rightCreated) return leftCreated - rightCreated; + return leftId.localeCompare(rightId); + }; + + const entries = [...groups.values()].flatMap((groupBindings) => { + const [selected] = [...groupBindings].sort(compareBindings); + const secret = secretsById.get(selected.binding.secretId); if (!secret) return []; - const accessBinding = secretBindings.find((binding) => binding.configPath.startsWith(AGENT_ACCESS_CONFIG_PATH_PREFIX)); - const selectedBinding = accessBinding ?? secretBindings[0]; - const hasEnv = secretBindings.some((binding) => binding.configPath.startsWith("env.")); - const hasApi = Boolean(accessBinding); - const versionSelector: SecretVersionSelector = selectedBinding.versionSelector === "latest" + const hasEnv = groupBindings.some((entry) => !entry.isApi); + const hasApi = groupBindings.some((entry) => entry.isApi); + const versionSelector: SecretVersionSelector = selected.binding.versionSelector === "latest" ? "latest" - : Number(selectedBinding.versionSelector); + : Number(selected.binding.versionSelector); const delivery: AgentSecretAccessEntry["delivery"] = hasEnv && hasApi ? "both" : hasEnv ? "env" : "api"; return [{ - secretId, - bindingId: selectedBinding.id, - configPath: selectedBinding.configPath, - key: secret.key, - name: secret.name, - description: secret.description ?? null, - delivery, - projectionClass: (selectedBinding.projectionClass ?? "unclassified") as SecretProjectionClass, - latestVersion: secret.latestVersion, - versionSelector, - resolvedVersion: versionSelector === "latest" ? secret.latestVersion : versionSelector, + entry: { + secretId: selected.binding.secretId, + bindingId: selected.binding.id, + configPath: selected.binding.configPath, + key: selected.key, + name: secret.name, + description: secret.description ?? null, + delivery, + projectionClass: (selected.binding.projectionClass ?? "unclassified") as SecretProjectionClass, + latestVersion: secret.latestVersion, + versionSelector, + resolvedVersion: versionSelector === "latest" ? secret.latestVersion : versionSelector, + } satisfies AgentSecretAccessEntry, + selected, }]; - }).sort((left, right) => left.key.localeCompare(right.key)); + }); + + // Two different secrets can still project the same metadata key (an alias that + // collides with another secret's key). Keep one entry per key so the value + // route resolves a single, deterministic binding. + const byKey = new Map(); + for (const candidate of entries) { + const existing = byKey.get(candidate.entry.key); + if (!existing || compareBindings(candidate.selected, existing.selected) < 0) { + byKey.set(candidate.entry.key, candidate); + } + } + + return [...byKey.values()] + .map(({ entry }) => entry) + .sort((left, right) => left.key.localeCompare(right.key)); } async function resolveSecretVersion(