From 492316e576bf55d84754f4669c0c0a2881da6bd5 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 9 Nov 2026 00:37:00 -0600 Subject: [PATCH] fix(plugins): accept secret_ref bindings in string secret-ref config fields (#20181) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Plugins declare their settings in an `instanceConfigSchema`, and the server validates each save against that schema > - Plugin credential fields use `type: "string", format: "secret-ref"` > - The settings form, the plugin secrets handler and the plugin SDK all use the `{ type: "secret_ref", secretId, version }` binding object for these fields, and the secrets handler rejects a bare secret id > - Schema validation ran on the raw config, so the binding object failed with "must be string" and no value could be saved > - This pull request validates the binding's secret id in place of the binding object > - The benefit is that an operator can bind a company secret to a plugin credential field from the settings page or the API ## Linked Issues or Issue Description **What happened** `POST /api/plugins/:pluginId/config` returned 400 "Configuration does not match the plugin's instanceConfigSchema" with `{"field":"/apiKey","message":"must be string"}` when the config held `{ "apiKey": { "type": "secret_ref", "secretId": "", "version": "latest" } }` for a field declared `{ "type": "string", "format": "secret-ref" }`. A bare UUID string passes the schema but the secrets handler then rejects it ("must use { type: \"secret_ref\", secretId, version? }"). So no value could be saved for a required secret-ref field. **Expected behavior** The binding object that the settings form submits and that `ctx.secrets.resolve()` accepts is saved. Other schema rules still apply. **Steps to reproduce** 1. Install a plugin whose manifest has `instanceConfigSchema.properties.apiKey = { "type": "string", "format": "secret-ref" }`. 2. Create a company secret. 3. On the plugin settings page, pick that secret for the field and click Save Configuration (or POST the binding object to `/api/plugins/:pluginId/config`). 4. See the 400 "must be string" response. Found while getting the chat eval cases `plugins.configure-plugin` and `plugins.disable-plugin` to run. Their setup and the human's settings-page save both hit this error. ## What Changed - `validateInstanceConfig` now replaces each valid `secret_ref` binding object at a schema `format: "secret-ref"` path with its secret id before Ajv validates. The stored config and the config sent to the worker do not change. - The walk follows array `items` (single and tuple) and objects inside arrays as well as object properties and `allOf`/`anyOf`/`oneOf`, so secret pickers inside array fields can save too. - A binding is accepted only when `envBindingSecretRefSchema` (the secrets handler's own parser) accepts it, so validation and the secrets handler agree on what is a valid binding. - Malformed bindings and objects in ordinary string fields still fail validation. - Added `server/src/__tests__/plugin-config-validator.test.ts`. ## Verification - `cd server && npx vitest run src/__tests__/plugin-config-validator.test.ts src/__tests__/plugin-routes-authz.test.ts src/__tests__/plugin-sandbox-provider-config-validation.test.ts`: 48 passed. - The new test fails on `master` without the fix (3 of 3 fail) and passes with it. - `cd server && npx vitest run src/__tests__/plugin-config-validator.test.ts`: 6 passed after the array/parser follow-up; the 3 follow-up tests fail on the previous commit. - `cd server && pnpm typecheck`: passes. ## Risks - Low risk. Only configs that held a valid binding object at a declared secret-ref path change result, from a 400 to accepted. The secrets handler still checks that the secret belongs to the company. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code execution in Claude Code. ## 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 - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 --- .../__tests__/plugin-config-validator.test.ts | 99 +++++++++++++++++++ .../src/services/plugin-config-validator.ts | 48 ++++++++- 2 files changed, 145 insertions(+), 2 deletions(-) create mode 100644 server/src/__tests__/plugin-config-validator.test.ts diff --git a/server/src/__tests__/plugin-config-validator.test.ts b/server/src/__tests__/plugin-config-validator.test.ts new file mode 100644 index 00000000000..e47eb8efe84 --- /dev/null +++ b/server/src/__tests__/plugin-config-validator.test.ts @@ -0,0 +1,99 @@ +import { describe, expect, it } from "vitest"; +import type { JsonSchema } from "@paperclipai/shared"; +import { validateInstanceConfig } from "../services/plugin-config-validator.js"; + +const SECRET_ID = "11111111-1111-4111-8111-111111111111"; +const binding = { type: "secret_ref", secretId: SECRET_ID, version: "latest" }; + +// The shape plugin manifests use: a string field marked as a secret reference. +const schema = { + type: "object", + properties: { + apiKey: { type: "string", title: "API key", format: "secret-ref" }, + region: { type: "string" }, + auth: { + type: "object", + properties: { token: { type: "string", format: "secret-ref" } }, + additionalProperties: false, + }, + }, + required: ["apiKey"], + additionalProperties: false, +} as unknown as JsonSchema; + +describe("validateInstanceConfig secret-ref fields", () => { + it("accepts the secret_ref binding object the settings form saves for a string secret-ref field", () => { + const configJson = { apiKey: binding, region: "us", auth: { token: { ...binding, version: 2 } } }; + expect(validateInstanceConfig(configJson, schema)).toEqual({ valid: true }); + // Validation does not rewrite what gets stored or sent to the worker. + expect(configJson.apiKey).toEqual(binding); + }); + + it("still enforces the rest of the schema around a binding", () => { + expect(validateInstanceConfig({ apiKey: binding, region: 5 }, schema)).toMatchObject({ + valid: false, + errors: [{ field: "/region" }], + }); + expect(validateInstanceConfig({ region: "us" }, schema).valid).toBe(false); + }); + + it("rejects objects that are not valid secret_ref bindings and objects in ordinary string fields", () => { + expect(validateInstanceConfig({ apiKey: { type: "secret_ref", secretId: "not-a-uuid" } }, schema)).toMatchObject({ + valid: false, + errors: [{ field: "/apiKey", message: "must be string" }], + }); + expect(validateInstanceConfig({ apiKey: binding, region: binding }, schema)).toMatchObject({ + valid: false, + errors: [{ field: "/region", message: "must be string" }], + }); + }); +}); + +describe("validateInstanceConfig secret-ref array items", () => { + const arraySchema = { + type: "object", + properties: { + keys: { type: "array", items: { type: "string", format: "secret-ref" } }, + accounts: { + type: "array", + items: { + type: "object", + properties: { name: { type: "string" }, token: { type: "string", format: "secret-ref" } }, + required: ["name", "token"], + additionalProperties: false, + }, + }, + }, + additionalProperties: false, + } as unknown as JsonSchema; + + it("accepts bindings the settings form saves in array items and nested array objects", () => { + const configJson = { + keys: [binding, { ...binding, version: 3 }], + accounts: [{ name: "primary", token: binding }], + }; + expect(validateInstanceConfig(configJson, arraySchema)).toEqual({ valid: true }); + expect(configJson.keys[0]).toEqual(binding); + expect(configJson.accounts[0]!.token).toEqual(binding); + }); + + it("still rejects malformed bindings and wrong types inside arrays", () => { + expect(validateInstanceConfig({ keys: [binding, { type: "secret_ref", secretId: "nope" }] }, arraySchema)).toMatchObject({ + valid: false, + errors: [{ field: "/keys/1", message: "must be string" }], + }); + expect(validateInstanceConfig({ accounts: [{ name: binding, token: binding }] }, arraySchema)).toMatchObject({ + valid: false, + errors: [{ field: "/accounts/0/name", message: "must be string" }], + }); + }); +}); + +describe("validateInstanceConfig secret-ref parsing matches the secrets handler", () => { + it("rejects a padded secret id that the secrets handler would also refuse", () => { + expect(validateInstanceConfig({ apiKey: { ...binding, secretId: ` ${SECRET_ID} ` } }, schema)).toMatchObject({ + valid: false, + errors: [{ field: "/apiKey", message: "must be string" }], + }); + }); +}); diff --git a/server/src/services/plugin-config-validator.ts b/server/src/services/plugin-config-validator.ts index 6eb95aa0fb2..404cc4e0041 100644 --- a/server/src/services/plugin-config-validator.ts +++ b/server/src/services/plugin-config-validator.ts @@ -10,13 +10,57 @@ import Ajv, { type ErrorObject } from "ajv"; import addFormats from "ajv-formats"; -import type { JsonSchema } from "@paperclipai/shared"; +import { envBindingSecretRefSchema, type JsonSchema } from "@paperclipai/shared"; export interface ConfigValidationResult { valid: boolean; errors?: { field: string; message: string }[]; } +/** + * Secret-ref fields are declared `type: "string"`, but the settings form and the + * plugin secrets API store the `{ type: "secret_ref", secretId, version }` binding + * object there. Validate the binding's secret id in its place so the schema + * checks the field without rejecting the only shape the secrets handler accepts. + * The walk follows object properties, array items and schema combinators, and + * accepts a binding only when the secrets handler's own parser accepts it. + */ +function secretRefsAsIds(value: unknown, schema: unknown): unknown { + if (!schema || typeof schema !== "object" || Array.isArray(schema)) return value; + const node = schema as Record; + + if (node.format === "secret-ref") { + const binding = envBindingSecretRefSchema.safeParse(value); + if (binding.success) return binding.data.secretId; + } + + let result = value; + for (const keyword of ["allOf", "anyOf", "oneOf"] as const) { + const branches = node[keyword]; + if (!Array.isArray(branches)) continue; + for (const branch of branches) result = secretRefsAsIds(result, branch); + } + + if (Array.isArray(result)) { + const items = node.items; + if (items && typeof items === "object") { + const itemSchemaAt = (index: number) => (Array.isArray(items) ? items[index] : items); + result = result.map((item, index) => secretRefsAsIds(item, itemSchemaAt(index))); + } + return result; + } + + const properties = node.properties; + if (result && typeof result === "object" && properties && typeof properties === "object") { + const record = { ...(result as Record) }; + for (const [key, propertySchema] of Object.entries(properties as Record)) { + if (key in record) record[key] = secretRefsAsIds(record[key], propertySchema); + } + result = record; + } + return result; +} + /** * Validate a config object against a JSON Schema. * @@ -39,7 +83,7 @@ export function validateInstanceConfig( // hint only — UUID validation happens in the secrets handler at resolve time. ajv.addFormat("secret-ref", { validate: () => true }); const validate = ajv.compile(schema); - const valid = validate(configJson); + const valid = validate(secretRefsAsIds(configJson, schema)); if (valid) { return { valid: true };