mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 02:26:12 +02:00
Clean up failed import secrets
This commit is contained in:
1 parent
07c820e613
commit
7c2827d33f
2 files changed
+115
No files matched your search
@@ -64,6 +64,7 @@ const assetSvc = {
|
||||
|
||||
const secretSvc = {
|
||||
create: vi.fn(async () => ({ id: "secret-created" })),
|
||||
remove: vi.fn(async () => ({ id: "secret-created" })),
|
||||
normalizeAdapterConfigForPersistence: vi.fn(async (_companyId: string, config: Record<string, unknown>) => config),
|
||||
normalizeEnvBindingsForPersistence: vi.fn(async (_companyId: string, env: Record<string, unknown>) => env),
|
||||
syncEnvBindingsForTarget: vi.fn(async () => []),
|
||||
@@ -133,6 +134,7 @@ describe("company portability", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
secretSvc.create.mockResolvedValue({ id: "secret-created" });
|
||||
secretSvc.remove.mockResolvedValue({ id: "secret-created" });
|
||||
secretSvc.normalizeAdapterConfigForPersistence.mockImplementation(async (_companyId, config) => config);
|
||||
secretSvc.normalizeEnvBindingsForPersistence.mockImplementation(async (_companyId, env) => env);
|
||||
secretSvc.syncEnvBindingsForTarget.mockResolvedValue([]);
|
||||
@@ -1501,6 +1503,74 @@ describe("company portability", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("cleans up unconsumed import secrets when agent import fails before persistence", async () => {
|
||||
const portability = companyPortabilityService({} as any);
|
||||
agentSvc.list.mockResolvedValue([]);
|
||||
agentSvc.create.mockRejectedValueOnce(new Error("agent create failed"));
|
||||
|
||||
await expect(portability.importBundle({
|
||||
source: {
|
||||
type: "inline",
|
||||
files: {
|
||||
"COMPANY.md": [
|
||||
"---",
|
||||
"name: Import",
|
||||
"includes:",
|
||||
" - agents/coder/AGENTS.md",
|
||||
"---",
|
||||
"",
|
||||
].join("\n"),
|
||||
"agents/coder/AGENTS.md": [
|
||||
"---",
|
||||
"name: Coder",
|
||||
"slug: coder",
|
||||
"kind: agent",
|
||||
"---",
|
||||
"",
|
||||
"# Coder",
|
||||
"",
|
||||
].join("\n"),
|
||||
".paperclip.yaml": [
|
||||
"schema: paperclip/v1",
|
||||
"agents:",
|
||||
" coder:",
|
||||
" adapter:",
|
||||
" type: codex_local",
|
||||
" config: {}",
|
||||
" inputs:",
|
||||
" env:",
|
||||
" OPENAI_API_KEY:",
|
||||
" kind: secret",
|
||||
" requirement: required",
|
||||
"",
|
||||
].join("\n"),
|
||||
},
|
||||
},
|
||||
include: {
|
||||
company: false,
|
||||
agents: true,
|
||||
projects: false,
|
||||
issues: false,
|
||||
},
|
||||
target: {
|
||||
mode: "existing_company",
|
||||
companyId: "company-1",
|
||||
},
|
||||
collisionStrategy: "rename",
|
||||
secretValues: {
|
||||
"agent:coder:OPENAI_API_KEY": "sk-imported",
|
||||
},
|
||||
}, "user-1")).rejects.toThrow("agent create failed");
|
||||
|
||||
expect(secretSvc.create).toHaveBeenCalledWith(
|
||||
"company-1",
|
||||
expect.objectContaining({ value: "sk-imported" }),
|
||||
{ userId: "user-1", agentId: null },
|
||||
);
|
||||
expect(secretSvc.remove).toHaveBeenCalledWith("secret-created");
|
||||
expect(secretSvc.syncEnvBindingsForTarget).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("exports project env as portable inputs without concrete values", async () => {
|
||||
const portability = companyPortabilityService({} as any);
|
||||
|
||||
|
||||
@@ -71,6 +71,7 @@ import { projectService } from "./projects.js";
|
||||
import { routineService } from "./routines.js";
|
||||
import { secretService } from "./secrets.js";
|
||||
import { getConfiguredSecretProvider } from "../secrets/configured-provider.js";
|
||||
import { logger } from "../middleware/logger.js";
|
||||
import {
|
||||
PORTABLE_CATALOG_PROVENANCE_STRING_KEYS,
|
||||
readCatalogStringList,
|
||||
@@ -2989,6 +2990,11 @@ export function companyPortabilityService(db: Db, storage?: StorageService) {
|
||||
const strictSecretsMode = process.env.PAPERCLIP_SECRETS_STRICT_MODE === "true";
|
||||
const defaultSecretProvider = getConfiguredSecretProvider();
|
||||
|
||||
type ImportSecretTracker = {
|
||||
createdSecretIds: string[];
|
||||
consumerPersisted: boolean;
|
||||
};
|
||||
|
||||
function assertKnownImportAdapterType(type: string | null | undefined): string {
|
||||
const adapterType = typeof type === "string" ? type.trim() : "";
|
||||
if (!adapterType) {
|
||||
@@ -3052,6 +3058,7 @@ export function companyPortabilityService(db: Db, storage?: StorageService) {
|
||||
envInputs: CompanyPortabilityEnvInput[],
|
||||
secretValues: Record<string, string> | null | undefined,
|
||||
actorUserId: string | null | undefined,
|
||||
secretTracker?: ImportSecretTracker,
|
||||
) {
|
||||
if (envInputs.length === 0) return;
|
||||
const missingRequired = envInputs.filter((input) => {
|
||||
@@ -3088,6 +3095,7 @@ export function companyPortabilityService(db: Db, storage?: StorageService) {
|
||||
},
|
||||
{ userId: actorUserId ?? null, agentId: null },
|
||||
);
|
||||
secretTracker?.createdSecretIds.push(secret.id);
|
||||
writeManifestEnvBinding(manifest, input, {
|
||||
type: "secret_ref",
|
||||
secretId: secret.id,
|
||||
@@ -3096,6 +3104,20 @@ export function companyPortabilityService(db: Db, storage?: StorageService) {
|
||||
}
|
||||
}
|
||||
|
||||
async function cleanupUnconsumedImportSecrets(secretTracker: ImportSecretTracker) {
|
||||
if (secretTracker.consumerPersisted || secretTracker.createdSecretIds.length === 0) return;
|
||||
for (const secretId of [...new Set(secretTracker.createdSecretIds)]) {
|
||||
try {
|
||||
await secrets.remove(secretId);
|
||||
} catch (err) {
|
||||
logger.warn(
|
||||
{ err, secretId },
|
||||
"failed to clean up unconsumed import secret after company import failure",
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function resolveImportedAssigneeAgentId(
|
||||
assigneeSlug: string | null | undefined,
|
||||
importedSlugToAgentId: Map<string, string>,
|
||||
@@ -4261,6 +4283,24 @@ export function companyPortabilityService(db: Db, storage?: StorageService) {
|
||||
input: CompanyPortabilityImport,
|
||||
actorUserId: string | null | undefined,
|
||||
options?: ImportBehaviorOptions,
|
||||
): Promise<CompanyPortabilityImportResult> {
|
||||
const secretTracker: ImportSecretTracker = {
|
||||
createdSecretIds: [],
|
||||
consumerPersisted: false,
|
||||
};
|
||||
try {
|
||||
return await importBundleInner(input, actorUserId, options, secretTracker);
|
||||
} catch (err) {
|
||||
await cleanupUnconsumedImportSecrets(secretTracker);
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
async function importBundleInner(
|
||||
input: CompanyPortabilityImport,
|
||||
actorUserId: string | null | undefined,
|
||||
options: ImportBehaviorOptions | undefined,
|
||||
secretTracker: ImportSecretTracker,
|
||||
): Promise<CompanyPortabilityImportResult> {
|
||||
const mode = resolveImportMode(options);
|
||||
const plan = await buildPreview(input, options);
|
||||
@@ -4391,6 +4431,7 @@ export function companyPortabilityService(db: Db, storage?: StorageService) {
|
||||
importEnvInputs,
|
||||
input.secretValues,
|
||||
actorUserId,
|
||||
secretTracker,
|
||||
);
|
||||
|
||||
if (include.company) {
|
||||
@@ -4565,6 +4606,7 @@ export function companyPortabilityService(db: Db, storage?: StorageService) {
|
||||
});
|
||||
continue;
|
||||
}
|
||||
secretTracker.consumerPersisted = true;
|
||||
try {
|
||||
const materialized = await instructions.materializeManagedBundle(updated, bundleFiles, {
|
||||
clearLegacyPromptTemplate: true,
|
||||
@@ -4597,6 +4639,7 @@ export function companyPortabilityService(db: Db, storage?: StorageService) {
|
||||
...patch,
|
||||
status: createdStatus,
|
||||
});
|
||||
secretTracker.consumerPersisted = true;
|
||||
await access.ensureMembership(targetCompany.id, "agent", created.id, "member", "active");
|
||||
await access.setPrincipalPermission(
|
||||
targetCompany.id,
|
||||
@@ -4724,6 +4767,7 @@ export function companyPortabilityService(db: Db, storage?: StorageService) {
|
||||
});
|
||||
continue;
|
||||
}
|
||||
secretTracker.consumerPersisted = true;
|
||||
projectId = updated.id;
|
||||
importedSlugToProjectId.set(planProject.slug, updated.id);
|
||||
existingProjectSlugToId.set(updated.urlKey, updated.id);
|
||||
@@ -4736,6 +4780,7 @@ export function companyPortabilityService(db: Db, storage?: StorageService) {
|
||||
});
|
||||
} else {
|
||||
const created = await projects.create(targetCompany.id, projectPatch);
|
||||
secretTracker.consumerPersisted = true;
|
||||
projectId = created.id;
|
||||
importedSlugToProjectId.set(planProject.slug, created.id);
|
||||
existingProjectSlugToId.set(created.urlKey, created.id);
|
||||
|
||||
Reference in new issue
Block a user