diff --git a/server/src/__tests__/agent-skills-routes.test.ts b/server/src/__tests__/agent-skills-routes.test.ts index 83352c4189..9cffa8f2a6 100644 --- a/server/src/__tests__/agent-skills-routes.test.ts +++ b/server/src/__tests__/agent-skills-routes.test.ts @@ -379,6 +379,54 @@ describe.sequential("agent skill routes", () => { ); }, 10_000); + it("lists skills without resolving required user-secret env bindings", async () => { + const adapterConfig = { + env: { + HOME: "/home/agent", + GH_TOKEN: { + type: "user_secret_ref" as const, + key: "github_pat_read_only", + version: "latest" as const, + required: true, + }, + }, + }; + mockAgentService.getById.mockResolvedValue({ + ...makeAgent("claude_local"), + adapterConfig, + }); + mockSecretService.resolveAdapterConfigForRuntime.mockImplementationOnce( + async ( + _companyId: string, + config: Record, + context?: unknown, + opts?: { skipUserSecrets?: boolean }, + ) => { + expect(config).toBe(adapterConfig); + expect(context).toBeUndefined(); + expect(opts).toEqual({ adapterType: "claude_local", skipUserSecrets: true }); + return { config: { env: { HOME: "/home/agent" } } }; + }, + ); + + const res = await requestApp( + await createApp(), + (baseUrl) => request(baseUrl) + .get("/api/agents/11111111-1111-4111-8111-111111111111/skills?companyId=company-1"), + ); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(mockAdapter.listSkills).toHaveBeenCalledWith( + expect.objectContaining({ + adapterType: "claude_local", + config: expect.objectContaining({ + env: { HOME: "/home/agent" }, + paperclipRuntimeSkills: expect.any(Array), + }), + }), + ); + }); + it("skips runtime materialization when listing Codex skills", async () => { mockAgentService.getById.mockResolvedValue(makeAgent("codex_local")); mockAdapter.listSkills.mockResolvedValue({ @@ -587,6 +635,61 @@ describe.sequential("agent skill routes", () => { expect(mockAdapter.syncSkills).toHaveBeenCalled(); }); + it("syncs skills without resolving required user-secret env bindings", async () => { + const adapterConfig = { + env: { + HOME: "/home/agent", + GH_TOKEN: { + type: "user_secret_ref" as const, + key: "github_pat_read_only", + version: "latest" as const, + required: true, + }, + }, + }; + mockAgentService.getById.mockResolvedValue({ + ...makeAgent("claude_local"), + adapterConfig, + }); + mockSecretService.resolveAdapterConfigForRuntime.mockImplementationOnce( + async ( + _companyId: string, + config: Record, + context?: unknown, + opts?: { skipUserSecrets?: boolean }, + ) => { + expect((config.env as Record).GH_TOKEN).toMatchObject({ + type: "user_secret_ref", + key: "github_pat_read_only", + }); + expect(context).toBeUndefined(); + expect(opts).toEqual({ adapterType: "claude_local", skipUserSecrets: true }); + return { + config: { + ...config, + env: { HOME: "/home/agent" }, + }, + }; + }, + ); + + const res = await requestApp(await createApp(), (baseUrl) => request(baseUrl) + .post("/api/agents/11111111-1111-4111-8111-111111111111/skills/sync?companyId=company-1") + .send({ desiredSkills: ["paperclipai/paperclip/paperclip"] })); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(mockAdapter.syncSkills).toHaveBeenCalledWith( + expect.objectContaining({ + adapterType: "claude_local", + config: expect.objectContaining({ + env: { HOME: "/home/agent" }, + paperclipRuntimeSkills: expect.any(Array), + }), + }), + ["paperclipai/paperclip/paperclip"], + ); + }); + it("canonicalizes desired skill references before syncing", async () => { mockAgentService.getById.mockResolvedValue(makeAgent("claude_local")); diff --git a/server/src/__tests__/secrets-service.test.ts b/server/src/__tests__/secrets-service.test.ts index 7dee6201f9..7f63cf11ee 100644 --- a/server/src/__tests__/secrets-service.test.ts +++ b/server/src/__tests__/secrets-service.test.ts @@ -616,6 +616,71 @@ describeEmbeddedPostgres("secretService", () => { expect(JSON.stringify(events)).not.toContain("user-one-secret"); }); + it("can skip user-secret refs while resolving adapter config for non-runtime skill discovery", async () => { + const companyId = await seedCompany(); + const svc = secretService(db); + await svc.createUserSecretDefinition(companyId, { + key: "github_token", + name: "GitHub token", + provider: "local_encrypted", + }); + const companySecret = await svc.create(companyId, { + name: `company-token-${randomUUID()}`, + provider: "local_encrypted", + value: "company-secret-value", + }); + const adapterConfig = { + apiBaseUrl: "http://127.0.0.1:9119/api", + apiKey: { + type: "user_secret_ref" as const, + key: "github_token", + version: "latest" as const, + required: true, + }, + env: { + HOME: "/home/agent", + COMPANY_TOKEN: { + type: "secret_ref" as const, + secretId: companySecret.id, + version: "latest" as const, + }, + GH_TOKEN: { + type: "user_secret_ref" as const, + key: "github_token", + version: "latest" as const, + required: true, + }, + }, + }; + + await expect( + svc.resolveAdapterConfigForRuntime(companyId, adapterConfig, undefined, { adapterType: "hermes_gateway" }), + ).rejects.toMatchObject({ + status: 422, + details: { code: "responsible_user_missing" }, + }); + + const resolved = await svc.resolveAdapterConfigForRuntime( + companyId, + adapterConfig, + undefined, + { adapterType: "hermes_gateway", skipUserSecrets: true }, + ); + + expect(resolved.config).not.toHaveProperty("apiKey"); + expect(resolved.config.env).toEqual({ + HOME: "/home/agent", + COMPANY_TOKEN: "company-secret-value", + }); + expect(resolved.secretKeys).toEqual(new Set(["COMPANY_TOKEN"])); + expect(resolved.manifest).toEqual([ + expect.objectContaining({ + secretId: companySecret.id, + outcome: "success", + }), + ]); + }); + it("returns conflict when concurrent user secret value creation races the unique index", async () => { const companyId = await seedCompany(); await seedCompanyMember(companyId, "user-1", "owner"); diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index 2d7c5a3e70..ec7faa32ea 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -1758,6 +1758,8 @@ export function agentRoutes( const { config: runtimeConfig } = await secretsSvc.resolveAdapterConfigForRuntime( agent.companyId, agent.adapterConfig, + undefined, + { adapterType: agent.adapterType, skipUserSecrets: true }, ); const runtimeSkillConfig = await buildRuntimeSkillConfig( agent.companyId, @@ -1824,6 +1826,8 @@ export function agentRoutes( const { config: runtimeConfig } = await secretsSvc.resolveAdapterConfigForRuntime( updated.companyId, updated.adapterConfig, + undefined, + { adapterType: updated.adapterType, skipUserSecrets: true }, ); const runtimeSkillConfig = { ...runtimeConfig, diff --git a/server/src/services/secrets.ts b/server/src/services/secrets.ts index 1a2454c2d5..ca84486cbc 100644 --- a/server/src/services/secrets.ts +++ b/server/src/services/secrets.ts @@ -406,6 +406,11 @@ type SecretResolutionOptions = { allowUserSecretScope?: boolean; }; +type ResolveAdapterConfigForRuntimeOptions = { + adapterType?: string | null; + skipUserSecrets?: boolean; +}; + export type RuntimeSecretManifestEntry = { configPath: string; envKey: string | null; @@ -4064,7 +4069,7 @@ export function secretService(db: Db) { companyId: string, adapterConfig: Record, context?: Omit, - opts?: { adapterType?: string | null }, + opts?: ResolveAdapterConfigForRuntimeOptions, ): Promise<{ config: Record; secretKeys: Set; manifest: RuntimeSecretManifestEntry[] }> => { const resolved = { ...adapterConfig }; const secretKeys = new Set(); @@ -4102,6 +4107,7 @@ export function secretService(db: Db) { manifest.push(secretResolution.manifestEntry); secretKeys.add(key); } else { + if (opts?.skipUserSecrets) continue; const secretResolution = await secretService(db).resolveUserSecretValue( companyId, { @@ -4135,6 +4141,10 @@ export function secretService(db: Db) { const binding = canonicalizeBinding(parsed.data as EnvBinding); if (binding.type === "plain") continue; if (binding.type === "user_secret_ref") { + if (opts?.skipUserSecrets) { + delete resolved[key]; + continue; + } const secretResolution = await secretService(db).resolveUserSecretValue( companyId, {