Fix top-level secret ref binding sync (#8630)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Instance environments can store provider configuration fields as Paperclip secret references > - Environment save uses secret binding sync to keep persisted config refs aligned with `company_secret_bindings` > - Top-level secret-ref fields such as `apiKey` were not deleted during sync because the cleanup only matched child paths like `apiKey.*` > - Re-saving an environment with the same top-level secret ref could therefore hit the target/path unique constraint and return a 500 > - This pull request makes the sync cleanup include the exact top-level config path before reinserting current refs > - The benefit is that saved environment provider configs can be edited repeatedly without duplicate binding failures ## Linked Issues or Issue Description No public GitHub issue found in duplicate search for this exact environment secret-binding failure. ### Bug report #### Pre-submission checklist - I searched existing open and closed issues and this is not a duplicate. - I can reproduce this on the current `master` lineage. - I confirmed the error originates in Paperclip secret-binding sync, not the sandbox provider itself. #### What happened? Saving an instance environment whose provider config contains a top-level secret-ref field can fail with a duplicate key error on `company_secret_bindings_target_path_uq`. #### Expected behavior Saving the same environment config repeatedly should update/sync bindings idempotently. #### Steps to reproduce 1. Create or edit an instance environment with a provider config that has a top-level secret-ref field such as `apiKey`. 2. Save the environment. 3. Save the environment again without moving that field under a nested object. 4. The second save can attempt to insert a duplicate binding for the same target/path. #### Paperclip version or commit Observed on local dev from current `master` lineage before this fix. #### Deployment mode Local dev (pnpm dev), authenticated private mode. #### Installation method Built from source (pnpm dev / pnpm build). #### Agent adapter(s) involved Not adapter-specific (core bug). #### Database mode Embedded local Postgres. #### Access context Board (human operator) environment settings save. #### Relevant logs or output The server returned a 500 after Postgres rejected a duplicate `company_secret_bindings` row for the same environment target and `apiKey` config path. Secret values and local paths are intentionally omitted. #### Privacy checklist I reviewed the PR description for private instance links, local paths, API keys, tokens, and company-specific secrets. ## What Changed - Updated `syncSecretRefsForTarget()` so prefix cleanup removes both the exact top-level config path and nested child paths. - Added a regression test that syncs an environment top-level `apiKey` secret ref repeatedly, then replaces it and verifies only one binding remains. ## Verification - `pnpm --filter @paperclipai/server exec vitest run src/__tests__/secrets-service.test.ts` - `pnpm --filter @paperclipai/server typecheck` - `git diff --check` - Local diff scan for internal issue links, local paths, bearer/session tokens, and obvious secret literals returned no matches. - GitHub duplicate searches for related environment secret-binding issues/PRs returned no matches. ## Risks Low risk. The change only broadens the existing target/path cleanup used before reinserting secret refs. It preserves the existing child-path cleanup behavior and adds the missing exact-path case. ## Model Used OpenAI GPT-5 Codex via the `codex_local` Paperclip adapter. Tool-using coding-agent session with shell, git, and repository-edit capabilities. Context window size was not exposed by the runtime. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
parent
841742fc1a
commit
f90ea4dae4
|
|
@ -145,6 +145,45 @@ describeEmbeddedPostgres("secretService", () => {
|
|||
).rejects.toThrow(/already exists/i);
|
||||
});
|
||||
|
||||
it("syncs top-level secret refs idempotently", async () => {
|
||||
const companyId = await seedCompany();
|
||||
const svc = secretService(db);
|
||||
const firstSecret = await svc.create(companyId, {
|
||||
name: `top-level-first-${randomUUID()}`,
|
||||
provider: "local_encrypted",
|
||||
value: "one",
|
||||
});
|
||||
const secondSecret = await svc.create(companyId, {
|
||||
name: `top-level-second-${randomUUID()}`,
|
||||
provider: "local_encrypted",
|
||||
value: "two",
|
||||
});
|
||||
const target = { targetType: "environment" as const, targetId: "env-1" };
|
||||
|
||||
await svc.syncSecretRefsForTarget(companyId, target, [
|
||||
{ secretId: firstSecret.id, configPath: "apiKey" },
|
||||
]);
|
||||
await svc.syncSecretRefsForTarget(companyId, target, [
|
||||
{ secretId: firstSecret.id, configPath: "apiKey" },
|
||||
]);
|
||||
await svc.syncSecretRefsForTarget(companyId, target, [
|
||||
{ secretId: secondSecret.id, configPath: "apiKey" },
|
||||
]);
|
||||
|
||||
const bindings = await db
|
||||
.select()
|
||||
.from(companySecretBindings)
|
||||
.where(eq(companySecretBindings.targetId, target.targetId));
|
||||
expect(bindings).toHaveLength(1);
|
||||
expect(bindings[0]).toMatchObject({
|
||||
companyId,
|
||||
targetType: "environment",
|
||||
targetId: target.targetId,
|
||||
configPath: "apiKey",
|
||||
secretId: secondSecret.id,
|
||||
});
|
||||
});
|
||||
|
||||
it("reports reference counts and resolves binding target labels", async () => {
|
||||
const companyId = await seedCompany();
|
||||
const svc = secretService(db);
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { and, desc, eq, inArray, like, ne, notInArray, sql } from "drizzle-orm";
|
||||
import { and, desc, eq, inArray, like, ne, notInArray, or, sql } from "drizzle-orm";
|
||||
import type { Db } from "@paperclipai/db";
|
||||
import {
|
||||
agents,
|
||||
|
|
@ -2190,7 +2190,10 @@ export function secretService(db: Db) {
|
|||
eq(companySecretBindings.companyId, companyId),
|
||||
eq(companySecretBindings.targetType, target.targetType),
|
||||
eq(companySecretBindings.targetId, target.targetId),
|
||||
like(companySecretBindings.configPath, `${pathPrefix}.%`),
|
||||
or(
|
||||
eq(companySecretBindings.configPath, pathPrefix),
|
||||
like(companySecretBindings.configPath, `${pathPrefix}.%`),
|
||||
),
|
||||
),
|
||||
);
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue