diff --git a/cli/src/commands/cloud.ts b/cli/src/commands/cloud.ts index 51dd6609d6..d0d49aa5df 100644 --- a/cli/src/commands/cloud.ts +++ b/cli/src/commands/cloud.ts @@ -11,6 +11,7 @@ import { registerJourneysCommands } from "./journeys.js"; import { registerOrganizationsCommands } from "./organizations.js"; import { registerProjectsCommands } from "./projects.js"; import { registerScenariosCommands } from "./scenarios.js"; +import { registerSecretsCommands } from "./secrets.js"; import { registerSessionsCommands } from "./sessions.js"; import { registerSwarmAuthoringCommands } from "./swarms.js"; import { registerTunnelCommands } from "./tunnel.js"; @@ -44,6 +45,9 @@ export function registerCloudCommands(program: Command): Command { registerEvalCommands(cloud); registerClientsCommands(cloud); registerEnvironmentsCommands(cloud); + // Project secrets sit with environments because an environment is what grants + // one to a run — you create a secret here and then select it there. + registerSecretsCommands(cloud); registerImagesCommands(cloud); registerSkillsCommands(cloud); diff --git a/cli/src/commands/secrets.ts b/cli/src/commands/secrets.ts new file mode 100644 index 0000000000..9676ea0241 --- /dev/null +++ b/cli/src/commands/secrets.ts @@ -0,0 +1,465 @@ +import { readFileSync } from "node:fs"; +import type { Command } from "commander"; +import { + createSecretOperation, + deleteSecretOperation, + getSecretOperation, + listSecretsOperation, + updateSecretOperation, +} from "@mcpjam/sdk/platform"; +import { usageError, writeResult } from "../lib/output.js"; +import { + platformOptionsOf, + runPlatformOperation as runPlatformCommand, + type PlatformOptions, +} from "../lib/platform-command.js"; +import { resolveCloudProjectArgs } from "../lib/cloud-scope.js"; +import { getGlobalOptions } from "../lib/server-config.js"; + +/** + * `mcpjam cloud secrets` — the project credentials a real workflow needs. + * + * A project secret is a named credential (`STRIPE_API_KEY`, `GH_TOKEN`, a + * `psql` password) that an environment can grant to the runs launched from it. + * + * ## Write-only + * + * Nothing here prints a value, and nothing can: `list` and `show` return + * metadata — name, delivery mode, host binding, sharing, when it was last + * handed to a run. A secret is written and delivered; it is never read back. + * + * ## How the value gets in + * + * `set` and `update` take the value from a FILE, an ENVIRONMENT VARIABLE, or + * STDIN. There is deliberately no positional argument for it: a credential + * typed as an argv token is written to shell history, is visible in `ps` and + * `/proc` to every process on the machine for the life of the command, and + * lands in CI logs that echo their commands. + * + * mcpjam cloud secrets set --name STRIPE_API_KEY --value-file ./key.txt + * mcpjam cloud secrets set --name STRIPE_API_KEY --value-env STRIPE_KEY + * pass show stripe | mcpjam cloud secrets set --name STRIPE_API_KEY --value - + * + * `--value ` exists, because scripting occasionally needs it, and it + * is documented as the scripting-only option with the history caveat attached + * rather than being quietly available. + * + * ## Delivery mode is a required decision + * + * --delivery brokered the sandbox's egress proxy injects the value as a + * request header, OUTSIDE the VM. The box never + * holds it, so a prompt-injected agent has nothing + * to exfiltrate. Prevents EXTRACTION, not USE — any + * process in the box can call the bound host while + * the policy is live — and works for HTTPS APIs + * only. Needs --host / --header / --template. + * --delivery materialized a real environment variable inside the box, which + * is the only thing a CLI can read. EXTRACTABLE BY + * DESIGN: `env` prints it. + */ + +/** Where a secret's value may come from. Exactly one, and never argv by default. */ +type ValueOptions = { + value?: string; + valueFile?: string; + valueEnv?: string; +}; + +/** + * Resolve the value from whichever source the caller named. + * + * `--value -` and `--value-file -` both read STDIN, so a pipeline reads + * naturally either way. The trailing newline a shell adds is stripped ONLY for + * stdin and file input, where it is an artifact of how the text was produced; + * an explicit `--value` is taken verbatim, because there the caller typed + * exactly what they meant. + */ +export function resolveSecretValue( + options: ValueOptions, + { required }: { required: boolean } +): string | undefined { + const supplied = [ + options.value !== undefined ? "--value" : null, + options.valueFile !== undefined ? "--value-file" : null, + options.valueEnv !== undefined ? "--value-env" : null, + ].filter((flag): flag is string => flag !== null); + + if (supplied.length > 1) { + throw usageError( + `Provide exactly one of --value, --value-file, or --value-env (got ${supplied.join( + ", " + )}).` + ); + } + if (supplied.length === 0) { + if (!required) return undefined; + throw usageError( + "A value is required. Prefer --value-file , --value-env , or `--value -` to read stdin; --value works but is written to your shell history." + ); + } + + if (options.valueEnv !== undefined) { + const value = process.env[options.valueEnv]; + if (value === undefined || value === "") { + throw usageError( + `Environment variable "${options.valueEnv}" is not set (or is empty).` + ); + } + return value; + } + + const readStdinOrFile = (path: string): string => { + try { + return path === "-" + ? readFileSync(0, "utf8") + : readFileSync(path, "utf8"); + } catch (error) { + throw usageError( + path === "-" + ? "Failed to read the secret value from stdin." + : `Failed to read the secret value from "${path}".`, + { source: error instanceof Error ? error.message : String(error) } + ); + } + }; + + // A FILE IS READ VERBATIM; ONLY STDIN LOSES ONE TRAILING NEWLINE. + // + // Stripping unconditionally was wrong, and wrong in the way the backend's own + // `validateSecretValue` warns about: a PEM block's final LF is part of the + // credential, so a `--value-file key.pem` that quietly dropped it stored a + // DIFFERENT secret than the file holds, and the failure surfaces much later + // as "the API key is wrong" with nothing to look at. The REST schema and + // `--value-env` both preserve whitespace; a file should agree with them. + // + // Stdin keeps the strip because there the newline is almost always the + // shell's rather than the credential's — `echo tok | mcpjam ...` is the + // dominant idiom — and it is documented on the flags as such. + const stripOneTrailingNewline = (text: string): string => + text.replace(/\r?\n$/, ""); + + if (options.valueFile !== undefined) { + const raw = readStdinOrFile(options.valueFile); + const text = options.valueFile === "-" ? stripOneTrailingNewline(raw) : raw; + if (text === "") throw usageError("The secret value is empty."); + return text; + } + + // `--value -` is the stdin spelling most people reach for first. + if (options.value === "-") { + const text = stripOneTrailingNewline(readStdinOrFile("-")); + if (text === "") throw usageError("The secret value is empty."); + return text; + } + if (options.value === "") throw usageError("The secret value is empty."); + return options.value; +} + +/** The broker triple, or nothing. Presence is checked against `--delivery`. */ +function brokerFields(options: { + host?: string[]; + header?: string; + template?: string; +}): { + brokerHosts?: string[]; + brokerHeader?: string; + brokerTemplate?: string; +} { + return { + ...(options.host !== undefined && options.host.length > 0 + ? { brokerHosts: options.host } + : {}), + ...(options.header !== undefined ? { brokerHeader: options.header } : {}), + ...(options.template !== undefined + ? { brokerTemplate: options.template } + : {}), + }; +} + +/** Commander's repeatable-option collector for `--host`. */ +function collectHost(value: string, previous: string[] = []): string[] { + return [...previous, value]; +} + +export function registerSecretsCommands(program: Command): void { + const secrets = program + .command("secrets") + .description( + "Store and manage the project credentials a workflow needs (stripe, gh, psql). Write-only: no command prints a value." + ); + + secrets + .command("list") + .description( + "List the project's secrets — metadata only. Shows the project-shared ones plus your own personal ones." + ) + .option( + "--project ", + "Project name or ID (defaults to the most recently updated project)" + ) + .action( + async (options: PlatformOptions & { project?: string }, command) => { + const globalOptions = getGlobalOptions(command); + const result = await runPlatformCommand( + platformOptionsOf(command), + globalOptions.timeout, + ({ client, signal }) => + listSecretsOperation.execute( + { project: resolveCloudProjectArgs(options).project }, + { client, signal } + ) + ); + writeResult(result, globalOptions.format); + } + ); + + secrets + .command("show") + .description( + "Show one secret's metadata: delivery mode, host binding, sharing, last delivery. Never its value." + ) + .requiredOption("--secret ", "Secret ID, from `cloud secrets list`") + .option("--project ", "Project name or ID") + .action( + async ( + options: PlatformOptions & { project?: string; secret: string }, + command + ) => { + const globalOptions = getGlobalOptions(command); + const result = await runPlatformCommand( + platformOptionsOf(command), + globalOptions.timeout, + ({ client, signal }) => + getSecretOperation.execute( + { + project: resolveCloudProjectArgs(options).project, + secret: options.secret, + }, + { client, signal } + ) + ); + writeResult(result, globalOptions.format); + } + ); + + secrets + .command("set") + .description( + "Create a secret. The value comes from --value-file, --value-env, or stdin — never a positional argument." + ) + .requiredOption( + "--name ", + "Environment-variable name (STRIPE_API_KEY). Uppercase, digits, underscores; not starting with a digit. Immutable." + ) + .requiredOption( + "--delivery ", + "brokered (the proxy injects it outside the sandbox — extraction-proof, not use-proof, HTTPS only) or materialized (a real env var inside the box, which is what a CLI can read, and which `env` prints)" + ) + .option( + "--value-file ", + "Read the value from a file, VERBATIM — a trailing newline is kept, because in a PEM block it is part of the credential. `-` reads stdin instead, which drops one trailing newline. Preferred." + ) + .option("--value-env ", "Read the value from an environment variable.") + .option( + "--value ", + "The value inline, or `-` to read stdin. SCRIPTING ONLY: an inline literal is written to your shell history and is visible in `ps` while the command runs." + ) + .option("--description ", "What this credential is for.") + .option( + "--host ", + "Brokered only, repeatable: an exact hostname the header is injected on (api.stripe.com). No scheme, no port, no wildcard.", + collectHost + ) + .option( + "--header ", + "Brokered only: the header name, e.g. Authorization." + ) + .option( + "--template ", + 'Brokered only: the header value with {} where the secret goes, e.g. "Bearer {}".' + ) + .option( + "--sharing ", + "project (default; delivered to every member's sessions, admin-only) or user (personal; delivered only in sessions you start)" + ) + .option( + "--idempotency-key ", + "Retry key. Pass one: a retried create without it fails as a name conflict with the row the first attempt already made." + ) + .action( + async ( + options: PlatformOptions & + ValueOptions & { + project?: string; + name: string; + delivery: string; + description?: string; + host?: string[]; + header?: string; + template?: string; + sharing?: string; + idempotencyKey?: string; + }, + command + ) => { + const globalOptions = getGlobalOptions(command); + const value = resolveSecretValue(options, { required: true })!; + // Validated through the operation's own schema, so the CLI and the API + // reject the same inputs with the same messages rather than growing a + // second, drifting copy of the rules. + const input = createSecretOperation.inputSchema.safeParse({ + project: resolveCloudProjectArgs(options).project, + name: options.name, + value, + ...(options.description !== undefined + ? { description: options.description } + : {}), + delivery: options.delivery, + ...brokerFields(options), + ...(options.sharing !== undefined + ? { sharing: options.sharing } + : {}), + ...(options.idempotencyKey !== undefined + ? { idempotencyKey: options.idempotencyKey } + : {}), + }); + if (!input.success) { + throw usageError( + `Invalid input: ${input.error.issues + .map( + (issue) => + `${issue.path.join(".") || "(root)"}: ${issue.message}` + ) + .join("; ")}` + ); + } + const result = await runPlatformCommand( + platformOptionsOf(command), + globalOptions.timeout, + ({ client, signal }) => + createSecretOperation.execute(input.data, { client, signal }) + ); + writeResult(result, globalOptions.format); + } + ); + + secrets + .command("update") + .description( + "Rotate a secret's value and/or change how it is delivered. A rotation reaches NEW RUNS ONLY — a session already running keeps the value it was given." + ) + .requiredOption("--secret ", "Secret ID, from `cloud secrets list`") + .option("--project ", "Project name or ID") + .option( + "--value-file ", + "Read the new value from a file, VERBATIM — a trailing newline is kept. `-` reads stdin instead, which drops one trailing newline. Preferred." + ) + .option( + "--value-env ", + "Read the new value from an environment variable." + ) + .option( + "--value ", + "The new value inline, or `-` to read stdin. SCRIPTING ONLY — see `secrets set`." + ) + .option("--description ", "Replacement description.") + .option( + "--clear-description", + "Remove the description entirely, leaving the secret with none." + ) + .option( + "--delivery ", + "brokered or materialized. Switching to brokered needs --host/--header/--template in the same call; switching to materialized clears them." + ) + .option("--host ", "Brokered only, repeatable.", collectHost) + .option("--header ", "Brokered only: the header name.") + .option("--template ", 'Brokered only: e.g. "Bearer {}".') + .action( + async ( + options: PlatformOptions & + ValueOptions & { + project?: string; + secret: string; + description?: string; + clearDescription?: boolean; + delivery?: string; + host?: string[]; + header?: string; + template?: string; + }, + command + ) => { + const globalOptions = getGlobalOptions(command); + // The two flags say opposite things about the same field, and picking a + // winner would silently discard half of what was asked for. + if (options.description !== undefined && options.clearDescription) { + throw usageError( + "Provide either --description or --clear-description, not both." + ); + } + const value = resolveSecretValue(options, { required: false }); + const input = updateSecretOperation.inputSchema.safeParse({ + project: resolveCloudProjectArgs(options).project, + secret: options.secret, + ...(value !== undefined ? { value } : {}), + // `--clear-description` is the flag spelling of the `null` the REST + // route and the SDK client already accept. Distinct from + // `--description ""`, which SETS an empty description. + ...(options.clearDescription ? { description: null } : {}), + ...(options.description !== undefined + ? { description: options.description } + : {}), + ...(options.delivery !== undefined + ? { delivery: options.delivery } + : {}), + ...brokerFields(options), + }); + if (!input.success) { + throw usageError( + `Invalid input: ${input.error.issues + .map( + (issue) => + `${issue.path.join(".") || "(root)"}: ${issue.message}` + ) + .join("; ")}` + ); + } + const result = await runPlatformCommand( + platformOptionsOf(command), + globalOptions.timeout, + ({ client, signal }) => + updateSecretOperation.execute(input.data, { client, signal }) + ); + writeResult(result, globalOptions.format); + } + ); + + secrets + .command("rm") + .description( + "Revoke a secret. HARD: the row and the encrypted value both go. Not blocked when an environment still selects it — revocation never waits on cleanup." + ) + .requiredOption("--secret ", "Secret ID, from `cloud secrets list`") + .option("--project ", "Project name or ID") + .action( + async ( + options: PlatformOptions & { project?: string; secret: string }, + command + ) => { + const globalOptions = getGlobalOptions(command); + const result = await runPlatformCommand( + platformOptionsOf(command), + globalOptions.timeout, + ({ client, signal }) => + deleteSecretOperation.execute( + { + project: resolveCloudProjectArgs(options).project, + secret: options.secret, + }, + { client, signal } + ) + ); + writeResult(result, globalOptions.format); + } + ); +} diff --git a/cli/src/lib/op-bindings.ts b/cli/src/lib/op-bindings.ts index 1246b990d2..db431728b1 100644 --- a/cli/src/lib/op-bindings.ts +++ b/cli/src/lib/op-bindings.ts @@ -84,6 +84,16 @@ export const CLI_BINDINGS: Readonly> = { update_persona: { command: "cloud personas update" }, delete_persona: { command: "cloud personas delete" }, generate_personas: { command: "cloud personas generate" }, + + // ── Project secrets ───────────────────────────────────────────────────── + // `set`/`update` take the value from --value-file, --value-env, or stdin. + // A positional value would be written to shell history and visible in `ps` + // for the life of the command, which is why the CLI does not offer one. + list_secrets: { command: "cloud secrets list" }, + get_secret: { command: "cloud secrets show" }, + create_secret: { command: "cloud secrets set" }, + update_secret: { command: "cloud secrets update" }, + delete_secret: { command: "cloud secrets rm" }, list_swarms: { command: "cloud swarms list" }, get_swarm: { command: "cloud swarms get" }, create_swarm: { command: "cloud swarms create" }, diff --git a/cli/tests/cloud-flag-conventions.test.ts b/cli/tests/cloud-flag-conventions.test.ts index 6ffa16c595..97f16f53d5 100644 --- a/cli/tests/cloud-flag-conventions.test.ts +++ b/cli/tests/cloud-flag-conventions.test.ts @@ -30,6 +30,7 @@ const CLOUD_COMMAND_FILES = [ "projects.ts", "registry.ts", "scenarios.ts", + "secrets.ts", "sessions.ts", "skills.ts", "swarms.ts", diff --git a/cli/tests/secrets-value-source.test.ts b/cli/tests/secrets-value-source.test.ts new file mode 100644 index 0000000000..70e3ef09b3 --- /dev/null +++ b/cli/tests/secrets-value-source.test.ts @@ -0,0 +1,59 @@ +/** + * Where a secret's bytes come from, and what is allowed to change them. + * + * The rule this pins: a FILE is read verbatim, and only STDIN loses one + * trailing newline. Stripping unconditionally stored a different secret than + * the file held — a PEM block's final LF is part of the credential — and the + * failure surfaces much later as "the API key is wrong" with nothing to look + * at. The REST schema and `--value-env` both preserve whitespace; a file has + * to agree with them. + * + * Stdin keeps the strip because there the newline is almost always the + * shell's, not the credential's: `echo tok | mcpjam …` is the dominant idiom. + */ +import assert from "node:assert/strict"; +import { mkdtemp, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import test from "node:test"; +import { resolveSecretValue } from "../src/commands/secrets.js"; + +const PEM = "-----BEGIN KEY-----\nabc\n-----END KEY-----\n"; + +async function fileWith(contents: string): Promise { + const dir = await mkdtemp(path.join(os.tmpdir(), "mcpjam-secret-")); + const file = path.join(dir, "value"); + await writeFile(file, contents, "utf8"); + return file; +} + +test("--value-file keeps a PEM block's trailing newline", async () => { + const file = await fileWith(PEM); + const value = resolveSecretValue({ valueFile: file }, { required: true }); + assert.equal(value, PEM); +}); + +test("--value-file keeps a lone trailing newline verbatim", async () => { + const file = await fileWith("sk_live_x\n"); + const value = resolveSecretValue({ valueFile: file }, { required: true }); + assert.equal(value, "sk_live_x\n"); +}); + +test("--value-file returns a newline-free value unchanged", async () => { + const file = await fileWith("sk_live_x"); + const value = resolveSecretValue({ valueFile: file }, { required: true }); + assert.equal(value, "sk_live_x"); +}); + +test("--value-env preserves whitespace, as it always did", () => { + process.env.MCPJAM_TEST_SECRET = PEM; + try { + const value = resolveSecretValue( + { valueEnv: "MCPJAM_TEST_SECRET" }, + { required: true } + ); + assert.equal(value, PEM); + } finally { + delete process.env.MCPJAM_TEST_SECRET; + } +}); diff --git a/docs/reference/openapi.json b/docs/reference/openapi.json index 41b367d03e..d19ae2fa53 100644 --- a/docs/reference/openapi.json +++ b/docs/reference/openapi.json @@ -5065,7 +5065,11 @@ "tags": ["Skills"], "summary": "List a project's skills", "description": "The Cloud Skills visible to the caller in this project: the project-shared ones plus the caller's own drafts. Each row reports `pinnability`, which is what decides whether its id is usable in an environment's `skillSelection`.", - "parameters": [{ "$ref": "#/components/parameters/projectId" }], + "parameters": [ + { + "$ref": "#/components/parameters/projectId" + } + ], "responses": { "200": { "description": "The project's skills.", @@ -5105,12 +5109,16 @@ "summary": "Get a skill", "description": "One skill, including its SKILL.md body. The body is mutable and an edit overwrites the previous one in place, so `aggregateHash` is the only handle on which content this read returned.", "parameters": [ - { "$ref": "#/components/parameters/projectId" }, + { + "$ref": "#/components/parameters/projectId" + }, { "name": "skillId", "in": "path", "required": true, - "schema": { "type": "string" }, + "schema": { + "type": "string" + }, "description": "Skill ID." } ], @@ -6633,6 +6641,233 @@ } } }, + "/projects/{projectId}/secrets": { + "parameters": [ + { + "$ref": "#/components/parameters/projectId" + } + ], + "get": { + "operationId": "listSecrets", + "tags": ["Secrets"], + "summary": "List secrets", + "description": "The project's credentials as METADATA ONLY — no value is returned by this or any other route. Shows the project-shared secrets plus the caller's own personal ones; another member's personal secret does not appear at all, not even its name.", + "responses": { + "200": { + "description": "A page of secrets.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SecretPage" + } + } + } + }, + "401": { + "$ref": "#/components/responses/Unauthorized" + }, + "403": { + "$ref": "#/components/responses/Forbidden" + }, + "404": { + "$ref": "#/components/responses/NotFound" + }, + "429": { + "$ref": "#/components/responses/RateLimited" + }, + "500": { + "$ref": "#/components/responses/InternalError" + } + } + }, + "post": { + "operationId": "createSecret", + "tags": ["Secrets"], + "summary": "Create a secret", + "description": "Store a credential so environments can grant it to runs. THE VALUE TRAVELS IN THE REQUEST BODY and becomes visible to whatever makes the call. The response is metadata only. Creating a project-shared secret requires project admin; a personal one does not.", + "parameters": [ + { + "name": "Idempotency-Key", + "in": "header", + "required": false, + "description": "Retry-safe create key. Replaying the SAME key with the SAME body returns the original resource instead of creating a second one; reusing it with a DIFFERENT body is a 409. Worth passing: a retried create without one fails as a name conflict with the row the first attempt already made.", + "schema": { + "type": "string" + } + } + ], + "requestBody": { + "required": true, + "description": "The secret to create.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SecretCreateRequest" + } + } + } + }, + "responses": { + "201": { + "description": "The created secret, as metadata.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Secret" + } + } + } + }, + "400": { + "$ref": "#/components/responses/ValidationError" + }, + "401": { + "$ref": "#/components/responses/Unauthorized" + }, + "403": { + "$ref": "#/components/responses/Forbidden" + }, + "404": { + "$ref": "#/components/responses/NotFound" + }, + "409": { + "$ref": "#/components/responses/Conflict" + }, + "429": { + "$ref": "#/components/responses/RateLimited" + }, + "500": { + "$ref": "#/components/responses/InternalError" + } + } + } + }, + "/projects/{projectId}/secrets/{secretId}": { + "parameters": [ + { + "$ref": "#/components/parameters/projectId" + }, + { + "$ref": "#/components/parameters/secretId" + } + ], + "get": { + "operationId": "getSecret", + "tags": ["Secrets"], + "summary": "Get a secret", + "description": "One secret's metadata: delivery mode, host binding, sharing, and when it was last handed to a run. Never its value. A secret from another project — and another member's personal secret — both read as 404.", + "responses": { + "200": { + "description": "The secret, as metadata.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Secret" + } + } + } + }, + "401": { + "$ref": "#/components/responses/Unauthorized" + }, + "403": { + "$ref": "#/components/responses/Forbidden" + }, + "404": { + "$ref": "#/components/responses/NotFound" + }, + "429": { + "$ref": "#/components/responses/RateLimited" + }, + "500": { + "$ref": "#/components/responses/InternalError" + } + } + }, + "patch": { + "operationId": "updateSecret", + "tags": ["Secrets"], + "summary": "Rotate or re-bind a secret", + "description": "Rotate the value and/or change how it is delivered. A rotation reaches NEW RUNS ONLY: a session already running holds the old value — materialized in its box's environment, or inside an egress policy that cannot be read back — and there is no safe way to replace it mid-run.", + "requestBody": { + "required": true, + "description": "The fields to change.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SecretUpdateRequest" + } + } + } + }, + "responses": { + "200": { + "description": "The updated secret, as metadata.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Secret" + } + } + } + }, + "400": { + "$ref": "#/components/responses/ValidationError" + }, + "401": { + "$ref": "#/components/responses/Unauthorized" + }, + "403": { + "$ref": "#/components/responses/Forbidden" + }, + "404": { + "$ref": "#/components/responses/NotFound" + }, + "409": { + "$ref": "#/components/responses/Conflict" + }, + "429": { + "$ref": "#/components/responses/RateLimited" + }, + "500": { + "$ref": "#/components/responses/InternalError" + } + } + }, + "delete": { + "operationId": "deleteSecret", + "tags": ["Secrets"], + "summary": "Delete a secret", + "description": "Revoke a credential. HARD — the row and the encrypted value both go. Deliberately NOT blocked when an environment still selects it: refusing would make a leaked credential un-revokable until someone edited every environment naming it, and revocation must never wait on cleanup.", + "responses": { + "200": { + "description": "The secret was revoked.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SecretDeleted" + } + } + } + }, + "401": { + "$ref": "#/components/responses/Unauthorized" + }, + "403": { + "$ref": "#/components/responses/Forbidden" + }, + "404": { + "$ref": "#/components/responses/NotFound" + }, + "429": { + "$ref": "#/components/responses/RateLimited" + }, + "500": { + "$ref": "#/components/responses/InternalError" + } + } + } + }, "/projects/{projectId}/journeys/generate": { "parameters": [ { @@ -11190,6 +11425,15 @@ "type": "string" }, "description": "Gate waiver ID, as returned when the waiver was granted or read." + }, + "secretId": { + "name": "secretId", + "in": "path", + "required": true, + "description": "Secret ID, as returned by the project's secret list.", + "schema": { + "type": "string" + } } }, "requestBodies": { @@ -17354,6 +17598,25 @@ } } }, + "EnvironmentSecretSelection": { + "type": "object", + "description": "Which PROJECT SECRETS a run launched from this environment receives — ids only. The environment is the GRANT BOUNDARY: absent means no secrets, and there is no \"all of them\" mode. Cannot be empty; clear the field instead (send `null` on update) to revoke the grant.\n\nMembership is not delivery. A `sharing: user` secret selected here reaches ONLY sessions its owner started; every other member's run of this environment silently does not receive it, and that rule is re-checked live at launch rather than baked into the selection.", + "required": ["mode", "secretIds"], + "properties": { + "mode": { + "type": "string", + "enum": ["explicit"] + }, + "secretIds": { + "type": "array", + "minItems": 1, + "items": { + "type": "string" + }, + "description": "Secret IDs, as returned by the project's secret list." + } + } + }, "ProjectEnvironment": { "type": "object", "description": "A project environment: a named, live-editable execution bundle that eval suites and journeys run against.", @@ -17396,6 +17659,9 @@ "skillSelection": { "$ref": "#/components/schemas/EnvironmentSkillSelection" }, + "secretSelection": { + "$ref": "#/components/schemas/EnvironmentSecretSelection" + }, "pluginVersionIds": { "type": "array", "items": { @@ -17701,7 +17967,9 @@ }, "SkillDetail": { "allOf": [ - { "$ref": "#/components/schemas/Skill" }, + { + "$ref": "#/components/schemas/Skill" + }, { "type": "object", "required": ["content"], @@ -17772,6 +18040,9 @@ "skillSelection": { "$ref": "#/components/schemas/EnvironmentSkillSelection" }, + "secretSelection": { + "$ref": "#/components/schemas/EnvironmentSecretSelection" + }, "pluginVersionIds": { "type": "array", "items": { @@ -17833,6 +18104,9 @@ "skillSelection": { "$ref": "#/components/schemas/EnvironmentSkillSelection" }, + "secretSelection": { + "$ref": "#/components/schemas/EnvironmentSecretSelection" + }, "pluginVersionIds": { "type": "array", "minItems": 1, @@ -17893,6 +18167,9 @@ "skillSelection": { "$ref": "#/components/schemas/EnvironmentSkillSelection" }, + "secretSelection": { + "$ref": "#/components/schemas/EnvironmentSecretSelection" + }, "pluginVersionIds": { "type": "array", "minItems": 1, @@ -17952,6 +18229,15 @@ "nullable": true, "description": "`null` clears the pinned skill selection." }, + "secretSelection": { + "allOf": [ + { + "$ref": "#/components/schemas/EnvironmentSecretSelection" + } + ], + "nullable": true, + "description": "`null` REVOKES the environment's credential grant; a value replaces it; omission leaves it unchanged. `[]` is rejected — an accidental empty array that read as \"remove every credential\" would break a workflow with no error to look at." + }, "pluginVersionIds": { "type": "array", "minItems": 1, @@ -19502,6 +19788,213 @@ } } }, + "Secret": { + "type": "object", + "description": "A project credential — METADATA ONLY, always. There is no `value` field on this schema and no route that returns one: a secret is written and delivered into a run, never read back.", + "required": [ + "id", + "projectId", + "name", + "description", + "delivery", + "sharing", + "lastDeliveredAt", + "createdAt", + "updatedAt", + "createdByUserId", + "updatedByUserId" + ], + "properties": { + "id": { + "type": "string" + }, + "projectId": { + "type": "string" + }, + "name": { + "type": "string", + "description": "The environment-variable name (`^[A-Z_][A-Z0-9_]*$`). This IS the secret's identity: what a materialized delivery exports, what a workflow references, and what stays stable across a rotation. Immutable." + }, + "description": { + "type": ["string", "null"] + }, + "delivery": { + "type": "string", + "enum": ["brokered", "materialized"], + "description": "`brokered` — the sandbox's egress proxy injects the value as a request header OUTSIDE the VM, so the box never holds it. Prevents EXTRACTION, not USE: any process in the box can call the bound host while the policy is live, and it works for HTTPS APIs only (domain rules bind on ports 80/443). `materialized` — a real environment variable inside the box, which is the only thing a CLI can read; EXTRACTABLE BY DESIGN." + }, + "brokerHosts": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Brokered only: the exact hostnames the header is injected on." + }, + "brokerHeader": { + "type": "string", + "description": "Brokered only: the header name." + }, + "brokerTemplate": { + "type": "string", + "description": "Brokered only: the header value, with `{}` where the secret goes." + }, + "sharing": { + "type": "string", + "enum": ["user", "project"], + "description": "`project` — admin-managed, delivered to every member's sessions. `user` — personal, delivered ONLY in sessions its owner starts and silently absent from anyone else's run of the same environment. Immutable." + }, + "ownerUserId": { + "type": "string", + "description": "Personal secrets only. Project-shared rows have no owner." + }, + "lastDeliveredAt": { + "type": ["integer", "null"], + "description": "When this secret was last HANDED TO a run — not when it was last used. Brokered use is unobservable by construction (the proxy injects the header; the request is never seen here), so `used` would be a number nobody can honestly produce. `null` means nothing has been recorded, which is not the same as never delivered." + }, + "createdAt": { + "type": "integer" + }, + "updatedAt": { + "type": "integer" + }, + "createdByUserId": { + "type": "string" + }, + "updatedByUserId": { + "type": "string" + } + } + }, + "SecretPage": { + "type": "object", + "required": ["items"], + "properties": { + "items": { + "type": "array", + "items": { + "$ref": "#/components/schemas/Secret" + } + }, + "nextCursor": { + "type": "string", + "description": "Present only when another page exists. Opaque — do not parse it." + } + } + }, + "SecretCreateRequest": { + "type": "object", + "description": "THE VALUE TRAVELS IN THIS BODY and becomes visible to whatever makes the call — its process, its logs, its shell history. Supply it from a file or an environment variable rather than pasting it into a command.", + "required": ["name", "value", "delivery"], + "properties": { + "name": { + "type": "string", + "maxLength": 64, + "pattern": "^[A-Z_][A-Z0-9_]*$", + "description": "Environment-variable name. Immutable — renaming is delete-and-recreate." + }, + "value": { + "type": "string", + "maxLength": 65536, + "description": "The credential. Stored encrypted; no route ever returns it. NOT trimmed — a trailing newline is meaningful in a PEM block, and rewriting what you sent would present as 'the key is wrong' with nothing to look at." + }, + "description": { + "type": "string", + "maxLength": 500 + }, + "delivery": { + "type": "string", + "enum": ["brokered", "materialized"], + "description": "Required, with no default: a caller who has not said whether the value ends up inside the sandbox has not made the decision this field exists for. See `Secret.delivery`." + }, + "brokerHosts": { + "type": "array", + "items": { + "type": "string" + }, + "minItems": 1, + "maxItems": 10, + "description": "Required for `brokered`, forbidden for `materialized`. Exact hostnames — no scheme, no port, no wildcard: the proxy matches a host, and a URL installs a rule that silently never fires." + }, + "brokerHeader": { + "type": "string", + "maxLength": 64, + "description": "Required for `brokered`, forbidden for `materialized`. e.g. `Authorization`." + }, + "brokerTemplate": { + "type": "string", + "maxLength": 256, + "description": "Required for `brokered`, forbidden for `materialized`. The header value with `{}` where the secret goes, e.g. `Bearer {}`. A template without `{}` is rejected: it installs a constant header that never carries the credential." + }, + "sharing": { + "type": "string", + "enum": ["user", "project"], + "default": "project", + "description": "Defaults to `project`. A non-admin asking for it is refused rather than downgraded to personal — a silent downgrade looks like success and then reaches nobody else's sessions." + } + } + }, + "SecretUpdateRequest": { + "type": "object", + "description": "At least one field. `name` and `sharing` are absent because both are IMMUTABLE: renaming would break the workflows referencing the environment variable, and re-sharing would change who has already been handed the value. Delete and recreate for either. A rotation reaches NEW RUNS ONLY — a session already running holds the old value and cannot be reached.", + "minProperties": 1, + "properties": { + "value": { + "type": "string", + "maxLength": 65536, + "description": "The new credential. Same exposure as on create: it travels in this body." + }, + "description": { + "type": ["string", "null"], + "maxLength": 500, + "description": "`null` clears it; omit to leave it unchanged." + }, + "delivery": { + "type": "string", + "enum": ["brokered", "materialized"], + "description": "Switching to `brokered` requires the host binding in the same call; switching to `materialized` clears it." + }, + "brokerHosts": { + "type": "array", + "items": { + "type": "string" + }, + "minItems": 1, + "maxItems": 10, + "description": "Required for `brokered`, forbidden for `materialized`. Exact hostnames — no scheme, no port, no wildcard: the proxy matches a host, and a URL installs a rule that silently never fires." + }, + "brokerHeader": { + "type": "string", + "maxLength": 64, + "description": "Required for `brokered`, forbidden for `materialized`. e.g. `Authorization`." + }, + "brokerTemplate": { + "type": "string", + "maxLength": 256, + "description": "Required for `brokered`, forbidden for `materialized`. The header value with `{}` where the secret goes, e.g. `Bearer {}`. A template without `{}` is rejected: it installs a constant header that never carries the credential." + } + } + }, + "SecretDeleted": { + "type": "object", + "description": "A HARD delete: the row and the encrypted value both go. This is the revoke button, so it is not soft.", + "required": ["id", "projectId", "name", "deleted"], + "properties": { + "id": { + "type": "string" + }, + "projectId": { + "type": "string" + }, + "name": { + "type": "string", + "description": "Echoed so a caller logging the revoke needs no prior read." + }, + "deleted": { + "type": "boolean", + "enum": [true] + } + } + }, "JourneyDraft": { "type": "object", "required": ["goal"], diff --git a/mcp/README.md b/mcp/README.md index ba8aa90b7f..c98de22627 100644 --- a/mcp/README.md +++ b/mcp/README.md @@ -110,6 +110,9 @@ so results respect the caller's project access. | `create_persona` | Create a reusable synthetic character for Swarms to run as. | — | | `update_persona` | Edit a persona's name, role or notes. Finished runs keep the persona they ran as. | — | | `delete_persona` | Remove a persona from the roster. Soft: history keeps resolving it. | — | +| `list_secrets` | List the project's credentials as metadata only — name, delivery mode, host binding, sharing. No value is ever returned. | — | +| `get_secret` | One secret's metadata: how it is delivered, where it is bound, when it was last handed to a run. Never its value. | — | +| `delete_secret` | Revoke a credential. Hard: the row and the encrypted value both go. | — | | `generate_personas` | Draft candidate personas with a model, grounded in what the project's servers do. Saves nothing; spends. | — | | `list_journeys` | List the project's journeys — a persona, a goal, and the environments to pursue it against. | — | | `get_journey` | Get one journey in full, including the execution config that determines how many sessions a run produces. | — | diff --git a/mcp/src/tools/platformTools.ts b/mcp/src/tools/platformTools.ts index e2fcbe6c4a..1f713d990c 100644 --- a/mcp/src/tools/platformTools.ts +++ b/mcp/src/tools/platformTools.ts @@ -97,6 +97,9 @@ import { createPersonaOperation, updatePersonaOperation, deletePersonaOperation, + listSecretsOperation, + getSecretOperation, + deleteSecretOperation, generatePersonasOperation, listJourneysOperation, getJourneyOperation, @@ -329,6 +332,13 @@ export const PLATFORM_CATALOG_OPERATIONS: ReadonlyArray< createPersonaOperation, updatePersonaOperation, deletePersonaOperation, + // PROJECT SECRETS — the metadata reads plus the revoke. The two write ops + // that carry a plaintext are in EXCLUDED_FROM_CATALOG; `delete_secret` is + // here because revoking a leaked credential is exactly the thing an + // unattended caller should be able to do without a human in the loop. + listSecretsOperation, + getSecretOperation, + deleteSecretOperation, generatePersonasOperation, listJourneysOperation, getJourneyOperation, @@ -490,24 +500,32 @@ export const EXCLUDED_FROM_CATALOG: Readonly> = { "Scenario exposure is already update_user_testing_scenario. The unified setter also changes who can open a conformance or eval share URL; shipping it now would add a second spelling of scenario mode on the unattended catalog.", rotate_share_link: "Scenario rotation is already rotate_user_testing_link. The unified rotate is destructive across resource types and should land with the same share group as the get/set pair, not as a third rotate tool.", + // PROJECT SECRET WRITES. Excluded for a reason that has nothing to do with + // how destructive they are, and everything to do with their INPUT: the + // plaintext credential is an argument, so it would transit model context and + // be written into chat transcripts before any approval card could render. + // An approval that runs after the value has already been logged is not an + // approval. The reads (list_secrets, get_secret) are in the catalog — they + // return metadata only and cannot produce a value. + create_secret: + "The plaintext value is an argument, so it would transit model context and be written into chat transcripts before any approval could run — an approval that fires after the credential is already logged is not one. Available on REST, the SDK and the CLI, where the caller controls where the value comes from. The metadata reads (list_secrets, get_secret) are in the catalog.", + update_secret: + "Same as create_secret: a rotation carries the new plaintext as an argument, so it would reach model context and the transcript before any approval could run. Available on REST, the SDK and the CLI. The metadata reads (list_secrets, get_secret) are in the catalog.", }; const catalogOperationNames = new Set( - PLATFORM_CATALOG_OPERATIONS.map((operation) => operation.name), + PLATFORM_CATALOG_OPERATIONS.map((operation) => operation.name) ); const allOperationNames = new Set( - ALL_OPERATIONS.map((operation) => operation.name), + ALL_OPERATIONS.map((operation) => operation.name) ); const staleCatalogExclusions = Object.keys(EXCLUDED_FROM_CATALOG).filter( - (name) => !allOperationNames.has(name), + (name) => !allOperationNames.has(name) ); const uncoveredCatalogOperations = ALL_OPERATIONS.filter( (operation) => !catalogOperationNames.has(operation.name) && - !Object.prototype.hasOwnProperty.call( - EXCLUDED_FROM_CATALOG, - operation.name, - ), + !Object.prototype.hasOwnProperty.call(EXCLUDED_FROM_CATALOG, operation.name) ); if ( staleCatalogExclusions.length > 0 || @@ -515,10 +533,10 @@ if ( ) { throw new Error( `Platform MCP catalog partition drift: stale=${staleCatalogExclusions.join( - ",", + "," )}; uncovered=${uncoveredCatalogOperations .map((operation) => operation.name) - .join(",")}`, + .join(",")}` ); } @@ -562,8 +580,8 @@ const DESTRUCTIVE_OPERATION_NAMES: ReadonlySet = new Set( ALL_OPERATIONS.filter( (operation) => operation.risk === "destructive" || - LEGACY_DESTRUCTIVE_NAMES.has(operation.name), - ).map((operation) => operation.name), + LEGACY_DESTRUCTIVE_NAMES.has(operation.name) + ).map((operation) => operation.name) ); /** @@ -585,6 +603,9 @@ const NON_IDEMPOTENT_DESTRUCTIVE_NAMES: ReadonlySet = new Set([ // not retryable), never looser. renderServerWidgetOperation.name, deletePersonaOperation.name, + // A HARD delete of a credential: the row and the ciphertext both go, and a + // second call cannot find the row to report the same outcome. + deleteSecretOperation.name, archiveJourneyOperation.name, archiveSwarmOperation.name, removeUserTestingMemberOperation.name, @@ -613,7 +634,7 @@ export const PLATFORM_TOOL_WIDGET_VIEWS: Readonly< export function registerPlatformCatalogTools( registrar: SessionToolRegistrar, - context: PlatformToolContext, + context: PlatformToolContext ): void { for (const operation of PLATFORM_CATALOG_OPERATIONS) { const view = PLATFORM_TOOL_WIDGET_VIEWS[operation.name]; @@ -626,7 +647,7 @@ export function registerPlatformCatalogTools( annotations: operationAnnotations(operation), }, async (input) => runPlatformOperation(context, operation, input), - view ? platformWidgetUi(context, operation, view) : undefined, + view ? platformWidgetUi(context, operation, view) : undefined ); } } @@ -641,7 +662,7 @@ export function registerPlatformCatalogTools( export function platformWidgetUi( context: PlatformToolContext, operation: PlatformOperation, - view: PlatformWidgetView, + view: PlatformWidgetView ) { return { resourceUri: PLATFORM_WIDGET_RESOURCE_URIS[view], @@ -654,13 +675,13 @@ export function platformWidgetUi( }, callback: async (input: unknown) => runPlatformOperation(context, operation, input, (payload) => - tagPlatformWidgetPayload(view, payload), + tagPlatformWidgetPayload(view, payload) ), }; } export function operationAnnotations( - operation: PlatformOperation, + operation: PlatformOperation ): ToolAnnotations { if (operation.readOnly) { return { readOnlyHint: true }; @@ -703,7 +724,7 @@ export function operationAnnotations( * before the call, not from the invoice. */ export function operationDescription( - operation: PlatformOperation, + operation: PlatformOperation ): string { return operation.risk === "spend" ? `${operation.description} COSTS MONEY: this consumes the organization's credits or configured provider keys.` @@ -714,7 +735,7 @@ export async function runPlatformOperation( context: PlatformToolContext, operation: PlatformOperation, input: TInput, - transformPayload?: (payload: TOutput) => object, + transformPayload?: (payload: TOutput) => object ) { // Resolve the bearer: the verified token for an authed session, or a // lazily-minted guest token for an anonymous one. Minting happens here (on @@ -761,7 +782,7 @@ export async function runPlatformOperation( } catch (error) { return toolError( describeOperationError(error), - errorStructuredContent(error), + errorStructuredContent(error) ); } } @@ -772,7 +793,7 @@ export async function runPlatformOperation( // calmly instead of with the alarming destructive styling. The model/CLI still // see `isError` plus the human-readable text message. function errorStructuredContent( - error: unknown, + error: unknown ): Record | undefined { if (isPlatformApiError(error)) { return { error: { code: error.code, message: error.message } }; @@ -948,7 +969,7 @@ function toolSuccess(payload: object, permalinks: PlatformPermalink[] = []) { function toolError( message: string, - structuredContent?: Record, + structuredContent?: Record ) { return { isError: true, diff --git a/mcp/tests/platformTools.test.ts b/mcp/tests/platformTools.test.ts index 74c55fc3da..7f7c28c277 100644 --- a/mcp/tests/platformTools.test.ts +++ b/mcp/tests/platformTools.test.ts @@ -212,6 +212,9 @@ const PLAIN_TOOLS = [ "create_persona", "update_persona", "delete_persona", + "list_secrets", + "get_secret", + "delete_secret", "generate_personas", "list_journeys", "get_journey", @@ -338,7 +341,9 @@ describe("platform tool registration", () => { registrations.map((registration) => [registration.name, registration]) ); for (const operation of PLATFORM_CATALOG_OPERATIONS) { - const description = String(byName.get(operation.name)?.config.description); + const description = String( + byName.get(operation.name)?.config.description + ); expect(description.includes("COSTS MONEY")).toBe( operation.risk === "spend" ); @@ -347,9 +352,9 @@ describe("platform tool registration", () => { expect(String(byName.get("run_eval_suite")?.config.description)).toContain( "COSTS MONEY" ); - expect(String(byName.get("list_eval_suites")?.config.description)).not.toContain( - "COSTS MONEY" - ); + expect( + String(byName.get("list_eval_suites")?.config.description) + ).not.toContain("COSTS MONEY"); }); it("registers show_servers with the MCP Apps UI resource", () => { @@ -460,6 +465,9 @@ describe("platform tool registration", () => { "create_persona", "update_persona", "delete_persona", + "list_secrets", + "get_secret", + "delete_secret", "generate_personas", "list_journeys", "get_journey", @@ -649,6 +657,9 @@ describe("platform tool registration", () => { // that running a third party's tool twice is safe. "render_server_widget", "delete_persona", + // A HARD credential revoke: the row and the ciphertext both go, so a + // second call cannot find the row to report the same outcome. + "delete_secret", "archive_journey", "archive_swarm", "remove_user_testing_member", @@ -669,6 +680,9 @@ describe("platform tool registration", () => { // roster and a second call answers not-found. From the caller's side // that is a removal. "delete_persona", + // Revoking a credential. Unlike the soft deletes around it, this one is + // genuinely irreversible — the encrypted value is gone. + "delete_secret", "archive_journey", "archive_swarm", "cancel_journey_run", diff --git a/mcpjam-inspector/client/src/components/ProjectSettingsTab.tsx b/mcpjam-inspector/client/src/components/ProjectSettingsTab.tsx index 6927c3ee37..ab1b08a4bc 100644 --- a/mcpjam-inspector/client/src/components/ProjectSettingsTab.tsx +++ b/mcpjam-inspector/client/src/components/ProjectSettingsTab.tsx @@ -6,6 +6,7 @@ import { AccountApiKeySection } from "./setting/AccountApiKeySection"; import { ProjectMembersFacepile } from "./project/ProjectMembersFacepile"; import { ProjectShareButton } from "./project/ProjectShareButton"; import { ProjectIconPicker } from "./project/ProjectEmojiPicker"; +import { ProjectSecretsSection } from "./project/ProjectSecretsSection"; import { Button } from "@mcpjam/design-system/button"; import { Input } from "@mcpjam/design-system/input"; @@ -82,7 +83,9 @@ function XaaTestDefaultsSection({ try { await onUpdateProject(projectId, { xaaTestDefaults: bothSet - ? { defaultIdentity: { subject: trimmedSubject, email: trimmedEmail } } + ? { + defaultIdentity: { subject: trimmedSubject, email: trimmedEmail }, + } : // Explicit clear — the mutation removes the stored default. null, }); @@ -106,8 +109,8 @@ function XaaTestDefaultsSection({ Identity provider: MCPJam test IdP - Used when an authenticated project member connects without a - server override. + Used when an authenticated project member connects without a server + override. {!hasStored && ( @@ -189,10 +192,7 @@ interface ProjectSettingsTabProps { updates: Partial, ) => Promise; onDeleteProject: (projectId: string) => Promise; - onProjectShared: ( - sharedProjectId: string, - sourceProjectId?: string, - ) => void; + onProjectShared: (sharedProjectId: string, sourceProjectId?: string) => void; onNavigateAway: () => void; } @@ -309,6 +309,18 @@ export function ProjectSettingsTab({ /> + {/* Project secrets — Convex-backed projects only: the store is a + Convex table, and a local project has nowhere to keep one. Shown to + every member rather than admins alone, because PERSONAL secrets are + owner-managed; `canManageShared` is what gates the project-shared + option inside the form. */} + {isAuthenticated && convexProjectId && ( + + )} + {/* XAA test identity defaults — Convex-backed projects only (the local-project update path is a no-op for this field). */} {isAuthenticated && convexProjectId && ( @@ -335,8 +347,8 @@ export function ProjectSettingsTab({ {isDefault ? "Switch to another project first" : !canDeleteProject - ? "Only project admins can delete this project" - : "Permanently delete this project and all its data"} + ? "Only project admins can delete this project" + : "Permanently delete this project and all its data"} @@ -361,8 +373,7 @@ export function ProjectSettingsTab({ Cancel { - const success = - await onDeleteProject(activeProjectId); + const success = await onDeleteProject(activeProjectId); if (success) { onNavigateAway(); } diff --git a/mcpjam-inspector/client/src/components/__tests__/ProjectSettingsTab.test.tsx b/mcpjam-inspector/client/src/components/__tests__/ProjectSettingsTab.test.tsx index 8bdf4a99b5..2d1740662f 100644 --- a/mcpjam-inspector/client/src/components/__tests__/ProjectSettingsTab.test.tsx +++ b/mcpjam-inspector/client/src/components/__tests__/ProjectSettingsTab.test.tsx @@ -26,6 +26,14 @@ vi.mock("convex/react", () => ({ }), })); +// The Secrets section is a sibling, not what these tests are about. Stubbed at +// the component boundary rather than by widening the `convex/react` mock: it +// reads a live query and drives three actions, and teaching this file about all +// four would make it a test of the secrets surface by accident. +vi.mock("@/components/project/ProjectSecretsSection", () => ({ + ProjectSecretsSection: () =>
, +})); + vi.mock("@workos-inc/authkit-react", () => ({ useAuth: () => ({ user: { email: "admin@example.com" } }), })); @@ -105,9 +113,7 @@ describe("ProjectSettingsTab — XAA test identity defaults", () => { const user = userEvent.setup(); const { onUpdateProject } = renderTab(); - expect( - screen.getByText("XAA test identity defaults"), - ).toBeInTheDocument(); + expect(screen.getByText("XAA test identity defaults")).toBeInTheDocument(); // Fixed issuer — the MCPJam test IdP, never enterprise SSO. expect( screen.getByText(/Identity provider: MCPJam test IdP/), diff --git a/mcpjam-inspector/client/src/components/environment-composer/environment-stack.ts b/mcpjam-inspector/client/src/components/environment-composer/environment-stack.ts index b45b68531a..b5a301ec49 100644 --- a/mcpjam-inspector/client/src/components/environment-composer/environment-stack.ts +++ b/mcpjam-inspector/client/src/components/environment-composer/environment-stack.ts @@ -21,6 +21,7 @@ * slot in keep today's one-axis compose. */ import type { + ProjectEnvironmentSecretSelection, ProjectEnvironmentSkillSelection, ProjectEnvironmentView, } from "@/hooks/useProjectEnvironments"; @@ -446,6 +447,28 @@ export function composerTargetCount(state: EnvironmentComposerState): number { return state.environmentIds.length; } +/** + * Two secret selections are the same grant. + * + * ORDER-SENSITIVE, matching `sameSkillSelection` and the backend's own + * order-preserving normalization: the stored array is what the fingerprint + * hashes, so two orderings are two rows and a comparison that called them equal + * would mark a real edit clean. + * + * Absent and null are the same thing (no grant); there is no empty-array case + * to reconcile, because a picker that clears its last row emits `null`. + */ +export function sameSecretSelection( + a: ProjectEnvironmentSecretSelection | null | undefined, + b: ProjectEnvironmentSecretSelection | null | undefined, +): boolean { + const left = a ?? null; + const right = b ?? null; + if (left === null || right === null) return left === right; + if (left.secretIds.length !== right.secretIds.length) return false; + return left.secretIds.every((id, index) => id === right.secretIds[index]); +} + export function sameSkillSelection( a: ProjectEnvironmentSkillSelection | null | undefined, b: ProjectEnvironmentSkillSelection | null | undefined, diff --git a/mcpjam-inspector/client/src/components/project-environments/ProjectEnvironmentEditor.tsx b/mcpjam-inspector/client/src/components/project-environments/ProjectEnvironmentEditor.tsx index 234409f5e9..af85a4015b 100644 --- a/mcpjam-inspector/client/src/components/project-environments/ProjectEnvironmentEditor.tsx +++ b/mcpjam-inspector/client/src/components/project-environments/ProjectEnvironmentEditor.tsx @@ -15,15 +15,20 @@ import { isRevisionConflictError, useCreateProjectEnvironment, useUpdateProjectEnvironment, + type ProjectEnvironmentSecretSelection, type ProjectEnvironmentSkillSelection, type ProjectEnvironmentView, } from "@/hooks/useProjectEnvironments"; import { ProjectEnvironmentSkillsPicker } from "./ProjectEnvironmentSkillsPicker"; +import { ProjectEnvironmentSecretsPicker } from "./ProjectEnvironmentSecretsPicker"; // Re-exported from the composer's shared helper rather than kept as a second // copy: a dirty-check that missed version pins would silently discard a "hold // this skill at v1" edit, and two implementations means one of them eventually // does. -import { sameSkillSelection } from "@/components/environment-composer/environment-stack"; +import { + sameSecretSelection, + sameSkillSelection, +} from "@/components/environment-composer/environment-stack"; type EnvironmentDraft = { name: string; @@ -31,6 +36,13 @@ type EnvironmentDraft = { hostId: string | null; serverAttachmentId: string | null; skillSelection: ProjectEnvironmentSkillSelection | null; + /** + * The environment's CREDENTIAL GRANT. Deliberately NOT flag-gated like skills + * and sandbox images: there is no `secrets-enabled` flag, and a picker that + * could vanish would be a picker whose stored grant could not be revoked from + * the UI. + */ + secretSelection: ProjectEnvironmentSecretSelection | null; computerEnvironmentId: string | null; }; @@ -45,6 +57,7 @@ function draftFromEnvironment(env: ProjectEnvironmentView): EnvironmentDraft { hostId: env.hostId, serverAttachmentId: env.serverAttachmentId ?? null, skillSelection: env.skillSelection ?? null, + secretSelection: env.secretSelection ?? null, computerEnvironmentId: env.computerEnvironmentId ?? null, }; } @@ -111,6 +124,7 @@ export function ProjectEnvironmentEditor({ hostId: null, serverAttachmentId: null, skillSelection: null, + secretSelection: null, computerEnvironmentId: null, ...initialDraft, }, @@ -141,6 +155,10 @@ export function ProjectEnvironmentEditor({ draft.skillSelection, environment.skillSelection ?? null, )) || + !sameSecretSelection( + draft.secretSelection, + environment.secretSelection ?? null, + ) || (computersEnabled && draft.computerEnvironmentId !== (environment.computerEnvironmentId ?? null)) @@ -149,6 +167,7 @@ export function ProjectEnvironmentEditor({ draft.hostId !== null || draft.serverAttachmentId !== null || (skillsEnabled && draft.skillSelection !== null) || + draft.secretSelection !== null || (computersEnabled && draft.computerEnvironmentId !== null); // Reactivity observed someone else's edit while this draft diverged. @@ -182,6 +201,10 @@ export function ProjectEnvironmentEditor({ hostId: null, serverAttachmentId: null, skillSelection: null, + // Dropped along with the rest: a grant naming the previous + // project's secrets would be rejected at save, and holding it + // would let a form submit ids the new project cannot resolve. + secretSelection: null, computerEnvironmentId: null, }, ); @@ -219,6 +242,9 @@ export function ProjectEnvironmentEditor({ ...(skillsEnabled && draft.skillSelection ? { skillSelection: draft.skillSelection } : {}), + ...(draft.secretSelection + ? { secretSelection: draft.secretSelection } + : {}), ...(computersEnabled && draft.computerEnvironmentId ? { computerEnvironmentId: draft.computerEnvironmentId } : {}), @@ -261,6 +287,17 @@ export function ProjectEnvironmentEditor({ ) ? { skillSelection: draft.skillSelection } : {}), + // NOT flag-gated, unlike the two fields around it — the picker is + // always rendered, so the "hidden picker must omit the field" rule has + // nothing to protect against here. Still tri-state: unchanged omits, + // and clearing the last selection sends `null`, which REVOKES the + // grant. + ...(!sameSecretSelection( + draft.secretSelection, + environment.secretSelection ?? null, + ) + ? { secretSelection: draft.secretSelection } + : {}), ...(computersEnabled && draft.computerEnvironmentId !== (environment.computerEnvironmentId ?? null) @@ -400,6 +437,18 @@ export function ProjectEnvironmentEditor({
) : null} +
+ + + setDraft((d) => ({ ...d, secretSelection })) + } + disabled={readOnly} + /> +
+ {computersEnabled ? (