mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-28 12:04:41 +01:00
Fix routine secret security blockers
This commit is contained in:
1 parent
74104940f5
commit
8fed992735
4 files changed
+203
-11
No files matched your search
@@ -104,6 +104,69 @@ describe("resolveExecutionRunAdapterConfig", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("drops Paperclip runtime-owned env before resolving agent, project, and routine overlays", async () => {
|
||||
const resolveAdapterConfigForRuntime = vi.fn(async (_companyId, config: Record<string, unknown>) => ({
|
||||
config: {
|
||||
...config,
|
||||
env: { ...(config.env as Record<string, unknown>) },
|
||||
},
|
||||
secretKeys: new Set<string>(),
|
||||
manifest: [],
|
||||
}));
|
||||
const resolveEnvBindings = vi.fn(async (_companyId, env: Record<string, unknown>) => ({
|
||||
env: Object.fromEntries(
|
||||
Object.entries(env).filter((entry): entry is [string, string] => typeof entry[1] === "string"),
|
||||
),
|
||||
secretKeys: new Set<string>(),
|
||||
manifest: [],
|
||||
}));
|
||||
|
||||
const result = await resolveExecutionRunAdapterConfig({
|
||||
companyId: "company-1",
|
||||
agentId: "agent-1",
|
||||
executionRunConfig: {
|
||||
env: {
|
||||
PAPERCLIP_API_KEY: { type: "secret_ref", secretId: "secret-api-key", version: "latest" },
|
||||
PAPERCLIP_AGENT_ID: "spoofed-agent",
|
||||
AGENT_ONLY: "agent-only",
|
||||
},
|
||||
},
|
||||
projectEnv: {
|
||||
PAPERCLIP_API_KEY: "project-api-key",
|
||||
PAPERCLIP_COMPANY_ID: "spoofed-company",
|
||||
PROJECT_ONLY: "project-only",
|
||||
},
|
||||
routineEnv: {
|
||||
PAPERCLIP_API_KEY: "routine-api-key",
|
||||
PAPERCLIP_RUN_ID: "spoofed-run",
|
||||
ROUTINE_ONLY: "routine-only",
|
||||
},
|
||||
routineId: "routine-1",
|
||||
secretsSvc: {
|
||||
resolveAdapterConfigForRuntime,
|
||||
resolveEnvBindings,
|
||||
} as any,
|
||||
});
|
||||
|
||||
expect(resolveAdapterConfigForRuntime.mock.calls[0]?.[1]).toEqual({
|
||||
env: {
|
||||
AGENT_ONLY: "agent-only",
|
||||
},
|
||||
});
|
||||
expect(resolveEnvBindings.mock.calls[0]?.[1]).toEqual({
|
||||
PROJECT_ONLY: "project-only",
|
||||
});
|
||||
expect(resolveEnvBindings.mock.calls[1]?.[1]).toEqual({
|
||||
ROUTINE_ONLY: "routine-only",
|
||||
});
|
||||
expect(result.resolvedConfig.env).toEqual({
|
||||
AGENT_ONLY: "agent-only",
|
||||
PROJECT_ONLY: "project-only",
|
||||
ROUTINE_ONLY: "routine-only",
|
||||
});
|
||||
expect(JSON.stringify(result.resolvedConfig.env)).not.toContain("PAPERCLIP_");
|
||||
});
|
||||
|
||||
it("skips project env resolution when the project has no bindings", async () => {
|
||||
const resolveAdapterConfigForRuntime = vi.fn().mockResolvedValue({
|
||||
config: { env: { AGENT_ONLY: "agent-only" } },
|
||||
|
||||
@@ -250,6 +250,71 @@ describeEmbeddedPostgres("secretService", () => {
|
||||
expect(JSON.stringify(events)).not.toContain("routine-super-secret");
|
||||
});
|
||||
|
||||
it("records stable redacted failure codes for routine env secret resolution", async () => {
|
||||
const companyId = await seedCompany();
|
||||
const svc = secretService(db);
|
||||
const secret = await svc.create(companyId, {
|
||||
name: `routine-failure-codes-${randomUUID()}`,
|
||||
provider: "local_encrypted",
|
||||
value: "routine-super-secret",
|
||||
});
|
||||
const env = {
|
||||
ROUTINE_API_KEY: { type: "secret_ref" as const, secretId: secret.id, version: "latest" as const },
|
||||
};
|
||||
const context = {
|
||||
consumerType: "routine" as const,
|
||||
consumerId: "routine-1",
|
||||
actorType: "agent" as const,
|
||||
actorId: "agent-1",
|
||||
};
|
||||
await svc.syncEnvBindingsForTarget(companyId, { targetType: "routine", targetId: "routine-1" }, env);
|
||||
|
||||
await expect(
|
||||
svc.resolveEnvBindings(companyId, env, { ...context, consumerId: "routine-2" }),
|
||||
).rejects.toThrow(/not bound/i);
|
||||
|
||||
await db.update(companySecrets).set({ status: "disabled" }).where(eq(companySecrets.id, secret.id));
|
||||
await expect(svc.resolveEnvBindings(companyId, env, context)).rejects.toThrow(/not active/i);
|
||||
|
||||
await db.update(companySecrets).set({ status: "active" }).where(eq(companySecrets.id, secret.id));
|
||||
await expect(
|
||||
svc.resolveSecretValue(companyId, secret.id, 999, {
|
||||
...context,
|
||||
configPath: "env.ROUTINE_API_KEY",
|
||||
}),
|
||||
).rejects.toThrow(/version not found/i);
|
||||
|
||||
await db
|
||||
.update(companySecretVersions)
|
||||
.set({ status: "disabled" })
|
||||
.where(eq(companySecretVersions.secretId, secret.id));
|
||||
await expect(svc.resolveEnvBindings(companyId, env, context)).rejects.toThrow(/version is not active/i);
|
||||
|
||||
await db
|
||||
.update(companySecretVersions)
|
||||
.set({ status: "current" })
|
||||
.where(eq(companySecretVersions.secretId, secret.id));
|
||||
vi.spyOn(localEncryptedProvider, "resolveVersion").mockRejectedValueOnce(
|
||||
new Error("provider leaked value routine-super-secret"),
|
||||
);
|
||||
await expect(svc.resolveEnvBindings(companyId, env, context)).rejects.toThrow(/provider leaked value/i);
|
||||
|
||||
await db.update(companySecrets).set({ status: "deleted" }).where(eq(companySecrets.id, secret.id));
|
||||
await expect(svc.resolveEnvBindings(companyId, env, context)).rejects.toThrow(/not found/i);
|
||||
|
||||
const events = await svc.listAccessEvents(companyId, secret.id);
|
||||
expect(events.map((event) => event.errorCode).sort()).toEqual([
|
||||
"binding_missing",
|
||||
"provider_error",
|
||||
"secret_deleted",
|
||||
"secret_inactive",
|
||||
"version_inactive",
|
||||
"version_missing",
|
||||
]);
|
||||
expect(JSON.stringify(events)).not.toContain("routine-super-secret");
|
||||
expect(JSON.stringify(events)).not.toContain("provider leaked value");
|
||||
});
|
||||
|
||||
it("scopes env binding sync deletes to the env path prefix", async () => {
|
||||
const companyId = await seedCompany();
|
||||
const svc = secretService(db);
|
||||
|
||||
@@ -331,6 +331,26 @@ type RuntimeConfigSecretResolver = Pick<
|
||||
"resolveAdapterConfigForRuntime" | "resolveEnvBindings"
|
||||
>;
|
||||
|
||||
function isPaperclipRuntimeEnvKey(key: string) {
|
||||
return key.startsWith("PAPERCLIP_");
|
||||
}
|
||||
|
||||
function stripPaperclipRuntimeEnvBindings(envValue: unknown): Record<string, unknown> | null {
|
||||
const record = parseObject(envValue);
|
||||
const filtered = Object.fromEntries(
|
||||
Object.entries(record).filter(([key]) => !isPaperclipRuntimeEnvKey(key)),
|
||||
);
|
||||
return Object.keys(filtered).length > 0 ? filtered : null;
|
||||
}
|
||||
|
||||
function stripPaperclipRuntimeEnvFromAdapterConfig(config: Record<string, unknown>): Record<string, unknown> {
|
||||
if (!Object.prototype.hasOwnProperty.call(config, "env")) return config;
|
||||
return {
|
||||
...config,
|
||||
env: stripPaperclipRuntimeEnvBindings(config.env) ?? {},
|
||||
};
|
||||
}
|
||||
|
||||
export async function resolveExecutionRunAdapterConfig(input: {
|
||||
companyId: string;
|
||||
agentId?: string | null;
|
||||
@@ -343,9 +363,12 @@ export async function resolveExecutionRunAdapterConfig(input: {
|
||||
routineEnv?: unknown;
|
||||
secretsSvc: RuntimeConfigSecretResolver;
|
||||
}) {
|
||||
const executionRunConfig = stripPaperclipRuntimeEnvFromAdapterConfig(input.executionRunConfig);
|
||||
const projectEnv = stripPaperclipRuntimeEnvBindings(input.projectEnv);
|
||||
const routineEnv = stripPaperclipRuntimeEnvBindings(input.routineEnv);
|
||||
const { config: resolvedConfig, secretKeys, manifest } = await input.secretsSvc.resolveAdapterConfigForRuntime(
|
||||
input.companyId,
|
||||
input.executionRunConfig,
|
||||
executionRunConfig,
|
||||
input.agentId
|
||||
? {
|
||||
consumerType: "agent",
|
||||
@@ -357,10 +380,10 @@ export async function resolveExecutionRunAdapterConfig(input: {
|
||||
}
|
||||
: undefined,
|
||||
);
|
||||
const projectEnvResolution = input.projectEnv
|
||||
const projectEnvResolution = projectEnv
|
||||
? await input.secretsSvc.resolveEnvBindings(
|
||||
input.companyId,
|
||||
input.projectEnv,
|
||||
projectEnv,
|
||||
input.projectId
|
||||
? {
|
||||
consumerType: "project",
|
||||
@@ -382,10 +405,10 @@ export async function resolveExecutionRunAdapterConfig(input: {
|
||||
secretKeys.add(key);
|
||||
}
|
||||
}
|
||||
const routineEnvResolution = input.routineEnv
|
||||
const routineEnvResolution = routineEnv
|
||||
? await input.secretsSvc.resolveEnvBindings(
|
||||
input.companyId,
|
||||
input.routineEnv,
|
||||
routineEnv,
|
||||
input.routineId
|
||||
? {
|
||||
consumerType: "routine",
|
||||
|
||||
@@ -195,6 +195,14 @@ type RuntimeSecretResolution = {
|
||||
manifestEntry: RuntimeSecretManifestEntry;
|
||||
};
|
||||
|
||||
type SecretResolutionErrorCode =
|
||||
| "binding_missing"
|
||||
| "secret_deleted"
|
||||
| "secret_inactive"
|
||||
| "version_missing"
|
||||
| "version_inactive"
|
||||
| "provider_error";
|
||||
|
||||
function asRecord(value: unknown): Record<string, unknown> | null {
|
||||
if (typeof value !== "object" || value === null || Array.isArray(value)) return null;
|
||||
return value as Record<string, unknown>;
|
||||
@@ -238,6 +246,33 @@ function defaultProviderConfigStatus(provider: SecretProvider): SecretProviderCo
|
||||
return COMING_SOON_SECRET_PROVIDERS.has(provider) ? "coming_soon" : "ready";
|
||||
}
|
||||
|
||||
function secretResolutionErrorCode(error: unknown): SecretResolutionErrorCode {
|
||||
if (isSecretProviderClientError(error)) return "provider_error";
|
||||
if (error instanceof HttpError) {
|
||||
const details = asRecord(error.details);
|
||||
switch (details?.code) {
|
||||
case "binding_missing":
|
||||
case "secret_deleted":
|
||||
case "secret_inactive":
|
||||
case "version_missing":
|
||||
case "version_inactive":
|
||||
case "provider_error":
|
||||
return details.code;
|
||||
}
|
||||
if (error.message === "Secret is not active") return "secret_inactive";
|
||||
if (error.message === "Secret version not found") return "version_missing";
|
||||
if (error.message === "Secret version is not active") return "version_inactive";
|
||||
if (
|
||||
error.message === "Secret resolution requires a binding config path" ||
|
||||
error.message.startsWith("Secret is not bound to ")
|
||||
) {
|
||||
return "binding_missing";
|
||||
}
|
||||
if (error.status >= 500) return "provider_error";
|
||||
}
|
||||
return "provider_error";
|
||||
}
|
||||
|
||||
function assertSelectableProviderConfig(config: {
|
||||
provider: string;
|
||||
status: string;
|
||||
@@ -321,7 +356,7 @@ export function secretService(db: Db) {
|
||||
) {
|
||||
if (!context) return;
|
||||
if (!context.configPath) {
|
||||
throw unprocessable("Secret resolution requires a binding config path");
|
||||
throw unprocessable("Secret resolution requires a binding config path", { code: "binding_missing" });
|
||||
}
|
||||
const binding = await getBinding({
|
||||
companyId,
|
||||
@@ -333,6 +368,7 @@ export function secretService(db: Db) {
|
||||
if (!binding) {
|
||||
throw unprocessable(
|
||||
`Secret is not bound to ${context.consumerType}:${context.consumerId} at ${context.configPath}`,
|
||||
{ code: "binding_missing" },
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -495,19 +531,24 @@ export function secretService(db: Db) {
|
||||
version: number | "latest",
|
||||
context?: SecretConsumerContext,
|
||||
): Promise<RuntimeSecretResolution> {
|
||||
const secret = await assertSecretInCompany(companyId, secretId);
|
||||
const secret = await getById(secretId);
|
||||
if (!secret) throw notFound("Secret not found");
|
||||
if (secret.companyId !== companyId) throw unprocessable("Secret must belong to same company");
|
||||
const resolvedVersion = version === "latest" ? secret.latestVersion : version;
|
||||
const providerId = secret.provider as SecretProvider;
|
||||
const configPath = context?.configPath ?? null;
|
||||
try {
|
||||
if (secret.status === "deleted") {
|
||||
throw new HttpError(404, "Secret not found", { code: "secret_deleted" });
|
||||
}
|
||||
if (secret.status !== "active") {
|
||||
throw unprocessable("Secret is not active");
|
||||
throw unprocessable("Secret is not active", { code: "secret_inactive" });
|
||||
}
|
||||
await assertBindingContext(companyId, secret.id, context);
|
||||
const versionRow = await getSecretVersion(secret.id, resolvedVersion);
|
||||
if (!versionRow) throw notFound("Secret version not found");
|
||||
if (!versionRow) throw new HttpError(404, "Secret version not found", { code: "version_missing" });
|
||||
if (versionRow.status === "disabled" || versionRow.status === "destroyed" || versionRow.revokedAt) {
|
||||
throw unprocessable("Secret version is not active");
|
||||
throw unprocessable("Secret version is not active", { code: "version_inactive" });
|
||||
}
|
||||
const provider = getSecretProvider(providerId);
|
||||
const providerConfig = await getSelectableRuntimeProviderConfig({
|
||||
@@ -555,7 +596,7 @@ export function secretService(db: Db) {
|
||||
},
|
||||
};
|
||||
} catch (err) {
|
||||
const errorCode = err instanceof Error ? err.message.slice(0, 120) : "resolution_failed";
|
||||
const errorCode = secretResolutionErrorCode(err);
|
||||
await recordAccessEvent({
|
||||
companyId,
|
||||
secretId: secret.id,
|
||||
|
||||
Reference in new issue
Block a user