Fix skills routes to skip user secret resolution

Skip user_secret_ref bindings when resolving adapter config for agent skill listing and sync paths, while keeping normal runtime resolution strict. Add route and service regression tests for required user-secret refs in adapter config.
This commit is contained in:
Nicky Leach 2026-07-09 15:09:58 -07:00 committed by GitHub
parent 8b6a06ee25
commit 4856558fd9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 183 additions and 1 deletions

View File

@ -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<string, unknown>,
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<string, unknown>,
context?: unknown,
opts?: { skipUserSecrets?: boolean },
) => {
expect((config.env as Record<string, unknown>).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"));

View File

@ -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");

View File

@ -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,

View File

@ -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<string, unknown>,
context?: Omit<SecretConsumerContext, "configPath">,
opts?: { adapterType?: string | null },
opts?: ResolveAdapterConfigForRuntimeOptions,
): Promise<{ config: Record<string, unknown>; secretKeys: Set<string>; manifest: RuntimeSecretManifestEntry[] }> => {
const resolved = { ...adapterConfig };
const secretKeys = new Set<string>();
@ -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,
{