From 548109b5fad42b940d79a98a33fde5761c41a6f8 Mon Sep 17 00:00:00 2001 From: Ethan Byrd Date: Mon, 11 May 2026 10:22:47 -0700 Subject: [PATCH 1/2] Add --secret KEY=VALUE flag to functions:deploy and functions:redeploy Repeatable flag that fans out into create + set-secret per pair + redeploy (deploy) or set-secret per pair + update (redeploy), so an agent can land first-deploy secrets in one command instead of the deploy, set, set, redeploy dance. --- .../src/oclif/commands/functions-deploy.ts | 375 ++++++++++++++- .../src/oclif/commands/functions-redeploy.ts | 238 +++++++++- cli-node/tests/oclif/functions-deploy.test.ts | 448 ++++++++++++++++++ .../tests/oclif/functions-redeploy.test.ts | 270 +++++++++++ 4 files changed, 1295 insertions(+), 36 deletions(-) create mode 100644 cli-node/tests/oclif/functions-deploy.test.ts create mode 100644 cli-node/tests/oclif/functions-redeploy.test.ts diff --git a/cli-node/src/oclif/commands/functions-deploy.ts b/cli-node/src/oclif/commands/functions-deploy.ts index 712175b..f4e0bf3 100644 --- a/cli-node/src/oclif/commands/functions-deploy.ts +++ b/cli-node/src/oclif/commands/functions-deploy.ts @@ -1,6 +1,15 @@ import { Command, Flags } from "@oclif/core"; -import type { CreateFunctionResult } from "@primitivedotdev/sdk/api"; -import { createFunction, PrimitiveApiClient } from "@primitivedotdev/sdk/api"; +import type { + CreateFunctionResult, + FunctionDetail, + FunctionSecretWriteResult, +} from "@primitivedotdev/sdk/api"; +import { + createFunction, + PrimitiveApiClient, + setFunctionSecret, + updateFunction, +} from "@primitivedotdev/sdk/api"; import { extractErrorPayload, readTextFileFlag, @@ -28,6 +37,249 @@ import { emitRawSendMailFetchWarning } from "../lint/raw-send-mail-fetch.js"; // // For full control (raw body, --raw-body JSON, etc.) the underlying // `functions:create-function` operation stays available. +// +// `--secret KEY=VALUE` is the one-call shortcut for "deploy a new +// function AND seed its secret bindings in the same command." The +// flag is repeatable. After the create step the CLI writes each +// secret in order, then re-deploys with the SAME bundle so the +// running handler picks up the bindings. Without secrets the flow +// is a single create call; with one or more --secret flags the +// flow fans out to (create + N set-secret + redeploy) API calls. + +// Server-side constraint on secret keys. Mirrored client-side so +// malformed input is rejected before any side-effecting API call. +const SECRET_KEY_RE = /^[A-Z_][A-Z0-9_]*$/; + +// Parsed --secret K=V pair. Exported so the unit test can build +// the same value the command produces internally. +export type SecretFlagPair = { key: string; value: string }; + +// Result of parsing the raw oclif --secret strings. Discriminated +// so the caller can decide whether to write a stderr error before +// touching the API surface. +export type ParseSecretFlagsResult = + | { kind: "ok"; secrets: SecretFlagPair[] } + | { kind: "error"; message: string }; + +// Split each `--secret KEY=VALUE` on the FIRST `=`. KEY must match +// `^[A-Z_][A-Z0-9_]*$`; VALUE may contain `=` (only the first one +// is treated as a delimiter). Returns a structured error rather +// than throwing so the caller can write the message to stderr in +// the same envelope shape used elsewhere. +export function parseSecretFlags(raw: string[]): ParseSecretFlagsResult { + const secrets: SecretFlagPair[] = []; + for (const entry of raw) { + const eq = entry.indexOf("="); + if (eq === -1) { + return { + kind: "error", + message: `--secret expects KEY=VALUE (got ${JSON.stringify(entry)}). Example: --secret API_TOKEN=abc123`, + }; + } + const key = entry.slice(0, eq); + const value = entry.slice(eq + 1); + if (key.length === 0) { + return { + kind: "error", + message: `--secret is missing a KEY before '=' (got ${JSON.stringify(entry)}). Example: --secret API_TOKEN=abc123`, + }; + } + if (!SECRET_KEY_RE.test(key)) { + return { + kind: "error", + message: `--secret KEY ${JSON.stringify(key)} does not match ${SECRET_KEY_RE.source} (uppercase letters, digits, underscores; first character is a letter or underscore).`, + }; + } + secrets.push({ key, value }); + } + return { kind: "ok", secrets }; +} + +// Final payload runDeployWithSecrets produces on the happy path. +// `created` is the initial create result; `redeploy` is the +// updateFunction return value after secrets were written. When +// secrets are present the CLI prints `redeploy` (the state the +// user actually deployed); when no secrets are passed only +// `created` is set. +export type DeployWithSecretsResult = { + created: CreateFunctionResult; + secrets?: FunctionSecretWriteResult[]; + redeploy?: FunctionDetail; +}; + +// Minimal client surface runDeployWithSecrets needs. Factored out +// so the unit test can pass a fake without standing up a real +// PrimitiveApiClient or the generated fetch stack. The real +// implementation in run() passes thin wrappers around the +// generated SDK functions. +export type DeployApiSurface = { + createFunction: (params: { + name: string; + code: string; + sourceMap?: string; + }) => Promise<{ + data?: { data?: CreateFunctionResult }; + error?: unknown; + }>; + setSecret: (params: { id: string; key: string; value: string }) => Promise<{ + data?: { data?: FunctionSecretWriteResult }; + error?: unknown; + }>; + updateFunction: (params: { + id: string; + code: string; + sourceMap?: string; + }) => Promise<{ + data?: { data?: FunctionDetail }; + error?: unknown; + }>; +}; + +// Discriminated result from runDeployWithSecrets. The caller +// surfaces either a success (with the final result payload) or an +// error stage that identifies which step failed so run() can write +// stage-specific stderr hints. `succeededKeys` / `failedKey` are +// populated for `set-secret` failures so the hint can list which +// keys landed before the failure. +export type RunDeployWithSecretsResult = + | { kind: "ok"; result: DeployWithSecretsResult } + | { + kind: "error"; + stage: "create"; + payload: unknown; + } + | { + kind: "error"; + stage: "set-secret"; + payload: unknown; + created: CreateFunctionResult; + succeededKeys: string[]; + failedKey: string; + } + | { + kind: "error"; + stage: "redeploy"; + payload: unknown; + created: CreateFunctionResult; + succeededKeys: string[]; + }; + +// Pure-ish orchestration of create + (optional secrets + redeploy). +// Mirrors runSetSecret in functions-set-secret.ts so the failure +// stages map directly onto stderr hints in run() below. Pulled out +// as a named export so the unit test can drive every branch with a +// fake DeployApiSurface, without spinning up a real client or the +// oclif command lifecycle. +export async function runDeployWithSecrets( + api: DeployApiSurface, + params: { + name: string; + code: string; + sourceMap?: string; + secrets: SecretFlagPair[]; + }, +): Promise { + const createResult = await api.createFunction({ + code: params.code, + name: params.name, + ...(params.sourceMap !== undefined ? { sourceMap: params.sourceMap } : {}), + }); + if (createResult.error) { + return { + kind: "error", + payload: extractErrorPayload(createResult.error), + stage: "create", + }; + } + const created = createResult.data?.data; + if (!created) { + return { + kind: "error", + payload: { + code: "client_error", + message: "Create function returned no data", + }, + stage: "create", + }; + } + + // Fast path: no secrets means no extra round-trips. The naked + // create result is exactly what the pre-secrets-flag command + // returned, so this branch is byte-identical to the previous + // behavior. + if (params.secrets.length === 0) { + return { kind: "ok", result: { created } }; + } + + const writtenSecrets: FunctionSecretWriteResult[] = []; + const succeededKeys: string[] = []; + for (const pair of params.secrets) { + const setResult = await api.setSecret({ + id: created.id, + key: pair.key, + value: pair.value, + }); + if (setResult.error) { + return { + created, + failedKey: pair.key, + kind: "error", + payload: extractErrorPayload(setResult.error), + stage: "set-secret", + succeededKeys, + }; + } + const secret = setResult.data?.data; + if (!secret) { + return { + created, + failedKey: pair.key, + kind: "error", + payload: { + code: "client_error", + message: "Secret write returned no data", + }, + stage: "set-secret", + succeededKeys, + }; + } + writtenSecrets.push(secret); + succeededKeys.push(pair.key); + } + + const updateResult = await api.updateFunction({ + code: params.code, + id: created.id, + ...(params.sourceMap !== undefined ? { sourceMap: params.sourceMap } : {}), + }); + if (updateResult.error) { + return { + created, + kind: "error", + payload: extractErrorPayload(updateResult.error), + stage: "redeploy", + succeededKeys, + }; + } + const redeployed = updateResult.data?.data; + if (!redeployed) { + return { + created, + kind: "error", + payload: { + code: "client_error", + message: "Redeploy returned no data", + }, + stage: "redeploy", + succeededKeys, + }; + } + + return { + kind: "ok", + result: { created, redeploy: redeployed, secrets: writtenSecrets }, + }; +} class FunctionsDeployCommand extends Command { static description = @@ -36,13 +288,26 @@ class FunctionsDeployCommand extends Command { Reads the bundle off disk (--file) instead of forcing the caller to serialize the source into a JSON body. Use the underlying operation \`functions:create-function\` if you need the full flag surface - (raw-body JSON, etc.).`; + (raw-body JSON, etc.). + + Pass --secret KEY=VALUE (repeatable) to seed secret bindings in the + same command. Keys must match \`^[A-Z_][A-Z0-9_]*$\` (uppercase + letters, digits, underscores; first character is a letter or + underscore). With one or more --secret flags the deploy fans out to + multiple API calls: create-function, set-secret per pair, then a + final update-function with the same bundle so the running handler + picks up the bindings. If a secret write fails after the create + step the function exists with whatever secrets succeeded and the + redeploy has NOT fired; re-run \`primitive functions:set-secret\` + for the missing keys, then \`primitive functions:redeploy\` to + push them live.`; static summary = "Deploy a new function from a bundled handler file"; static examples = [ "<%= config.bin %> functions:deploy --name forwarder --file ./bundle.js", "<%= config.bin %> functions:deploy --name forwarder --file ./bundle.js --source-map-file ./bundle.js.map", + "<%= config.bin %> functions:deploy --name forwarder --file ./bundle.js --secret OPENAI_KEY=sk-... --secret OWNER_EMAIL=me@example.com", ]; static flags = { @@ -77,6 +342,11 @@ class FunctionsDeployCommand extends Command { description: "Optional path to a source map for the bundle. Stored only on the runtime side and used to symbolicate stack traces.", }), + secret: Flags.string({ + description: + "Secret KEY=VALUE to seed on the deployed function. Repeatable. KEY must match `^[A-Z_][A-Z0-9_]*$`; VALUE may contain `=` (only the first `=` is treated as a delimiter). Passing one or more --secret flags fans out the deploy to create-function, set-secret per pair, then a final redeploy so the running handler picks up the bindings.", + multiple: true, + }), time: Flags.boolean({ description: TIME_FLAG_DESCRIPTION, }), @@ -86,6 +356,18 @@ class FunctionsDeployCommand extends Command { const { flags } = await this.parse(FunctionsDeployCommand); await runWithTiming(flags.time, async () => { + // Validate --secret pairs BEFORE any disk read or API call so + // a malformed input fails fast with a clear error and zero + // side effects. The fast path (no --secret flags) skips this + // entirely. + const rawSecrets = flags.secret ?? []; + const parsedSecrets = parseSecretFlags(rawSecrets); + if (parsedSecrets.kind === "error") { + process.stderr.write(`${parsedSecrets.message}\n`); + process.exitCode = 1; + return; + } + // Reads are inside the timed block so --time captures disk I/O // alongside the API call. A pathological filesystem (NFS, slow // FUSE mount) showing up here is exactly the kind of latency @@ -123,31 +405,86 @@ class FunctionsDeployCommand extends Command { configDir: this.config.configDir, }; - const result = await createFunction({ - body: { - name: flags.name, - code, - ...(sourceMap !== undefined ? { sourceMap } : {}), - }, - client: apiClient.client, - responseStyle: "fields", + // Adapter: thin wrappers around the generated SDK calls, + // routed through host 1 (apiClient.client). The function + // CRUD and secrets endpoints are not on host 2. + const apiSurface: DeployApiSurface = { + createFunction: (p) => + createFunction({ + body: { + code: p.code, + name: p.name, + ...(p.sourceMap !== undefined ? { sourceMap: p.sourceMap } : {}), + }, + client: apiClient.client, + responseStyle: "fields", + }), + setSecret: (p) => + setFunctionSecret({ + body: { value: p.value }, + client: apiClient.client, + path: { id: p.id, key: p.key }, + responseStyle: "fields", + }), + updateFunction: (p) => + updateFunction({ + body: { + code: p.code, + ...(p.sourceMap !== undefined ? { sourceMap: p.sourceMap } : {}), + }, + client: apiClient.client, + path: { id: p.id }, + responseStyle: "fields", + }), + }; + + const outcome = await runDeployWithSecrets(apiSurface, { + code, + name: flags.name, + secrets: parsedSecrets.secrets, + ...(sourceMap !== undefined ? { sourceMap } : {}), }); - if (result.error) { - const errorPayload = extractErrorPayload(result.error); - writeErrorWithHints(errorPayload); + if (outcome.kind === "error") { + // Stage-specific framing on stderr so callers can tell + // whether the function was created before a downstream + // failure left it without secrets or without the + // redeploy. The JSON envelope still goes through + // writeErrorWithHints so any actionable hint (e.g. + // unauthorized) is surfaced. + if (outcome.stage === "set-secret") { + const succeeded = + outcome.succeededKeys.length > 0 + ? outcome.succeededKeys.join(", ") + : "(none)"; + process.stderr.write( + `Function ${outcome.created.name} (${outcome.created.id}) was created, but writing secret ${outcome.failedKey} failed; succeeded keys so far: ${succeeded}. The redeploy is NOT yet live. Re-run \`primitive functions:set-secret --id ${outcome.created.id} --key ${outcome.failedKey} --value --redeploy\` after fixing the cause.\n`, + ); + } else if (outcome.stage === "redeploy") { + const succeeded = + outcome.succeededKeys.length > 0 + ? outcome.succeededKeys.join(", ") + : "(none)"; + process.stderr.write( + `Function ${outcome.created.name} (${outcome.created.id}) was created and secrets [${succeeded}] were written, but the final redeploy failed; the new bindings are NOT yet live. Re-run \`primitive functions:redeploy --id ${outcome.created.id} --file \` once the cause is fixed.\n`, + ); + } + writeErrorWithHints(outcome.payload); removeStaleSavedCredentialOnUnauthorized({ ...authFailureContext, - payload: errorPayload, + payload: outcome.payload, }); process.exitCode = 1; return; } - const envelope = result.data as - | { data?: CreateFunctionResult } - | undefined; - this.log(JSON.stringify(envelope?.data ?? null, null, 2)); + // On the happy path, prefer the redeployed FunctionDetail + // (when secrets fired) over the bare CreateFunctionResult, + // since the redeploy is the state the user actually + // deployed. When no secrets were passed, fall back to the + // create payload for byte-identical pre-flag behavior. + const payload = outcome.result.redeploy ?? outcome.result.created; + this.log(JSON.stringify(payload, null, 2)); }); } } diff --git a/cli-node/src/oclif/commands/functions-redeploy.ts b/cli-node/src/oclif/commands/functions-redeploy.ts index 27f3771..f4d0982 100644 --- a/cli-node/src/oclif/commands/functions-redeploy.ts +++ b/cli-node/src/oclif/commands/functions-redeploy.ts @@ -1,6 +1,13 @@ import { Command, Flags } from "@oclif/core"; -import type { FunctionDetail } from "@primitivedotdev/sdk/api"; -import { PrimitiveApiClient, updateFunction } from "@primitivedotdev/sdk/api"; +import type { + FunctionDetail, + FunctionSecretWriteResult, +} from "@primitivedotdev/sdk/api"; +import { + PrimitiveApiClient, + setFunctionSecret, + updateFunction, +} from "@primitivedotdev/sdk/api"; import { extractErrorPayload, readTextFileFlag, @@ -11,6 +18,7 @@ import { } from "../api-command.js"; import { resolveCliAuth } from "../auth.js"; import { emitRawSendMailFetchWarning } from "../lint/raw-send-mail-fetch.js"; +import { parseSecretFlags, type SecretFlagPair } from "./functions-deploy.js"; // `primitive functions:redeploy` is the agent-grade shortcut for // `functions:update-function`. Same file-reading ergonomic as @@ -19,6 +27,142 @@ import { emitRawSendMailFetchWarning } from "../lint/raw-send-mail-fetch.js"; // previously-deployed bundle (or any equivalent file) re-runs the // deploy and refreshes env from the secrets table, which is how // secret writes go live. +// +// `--secret KEY=VALUE` is the one-call shortcut for "write secrets +// AND redeploy in the same command." The flag is repeatable. Secrets +// are written FIRST so a single update-function call picks up every +// new binding; this is the inverse order of functions:deploy where +// the function must exist before secrets can be written. + +// Final payload runRedeployWithSecrets produces on the happy path. +// `secrets` is omitted when no --secret flags were passed. +export type RedeployWithSecretsResult = { + redeploy: FunctionDetail; + secrets?: FunctionSecretWriteResult[]; +}; + +// Minimal client surface runRedeployWithSecrets needs. Mirrors +// DeployApiSurface but without createFunction since the function +// already exists. +export type RedeployApiSurface = { + setSecret: (params: { id: string; key: string; value: string }) => Promise<{ + data?: { data?: FunctionSecretWriteResult }; + error?: unknown; + }>; + updateFunction: (params: { + id: string; + code: string; + sourceMap?: string; + }) => Promise<{ + data?: { data?: FunctionDetail }; + error?: unknown; + }>; +}; + +// Discriminated result from runRedeployWithSecrets. The caller +// surfaces either a success or an error stage so run() can write +// stage-specific stderr hints. `succeededKeys` / `failedKey` are +// populated for `set-secret` failures so the hint can list which +// keys landed before the failure. +export type RunRedeployWithSecretsResult = + | { kind: "ok"; result: RedeployWithSecretsResult } + | { + kind: "error"; + stage: "set-secret"; + payload: unknown; + succeededKeys: string[]; + failedKey: string; + } + | { + kind: "error"; + stage: "redeploy"; + payload: unknown; + succeededKeys: string[]; + }; + +// Pure-ish orchestration of (optional secrets +) update-function. +// Writes every secret first, then re-deploys with the new bundle so +// a single updateFunction call refreshes every binding the user +// wrote. Pulled out as a named export so the unit test can drive +// every branch with a fake RedeployApiSurface, without spinning up +// a real client or the oclif command lifecycle. +export async function runRedeployWithSecrets( + api: RedeployApiSurface, + params: { + id: string; + code: string; + sourceMap?: string; + secrets: SecretFlagPair[]; + }, +): Promise { + const writtenSecrets: FunctionSecretWriteResult[] = []; + const succeededKeys: string[] = []; + for (const pair of params.secrets) { + const setResult = await api.setSecret({ + id: params.id, + key: pair.key, + value: pair.value, + }); + if (setResult.error) { + return { + failedKey: pair.key, + kind: "error", + payload: extractErrorPayload(setResult.error), + stage: "set-secret", + succeededKeys, + }; + } + const secret = setResult.data?.data; + if (!secret) { + return { + failedKey: pair.key, + kind: "error", + payload: { + code: "client_error", + message: "Secret write returned no data", + }, + stage: "set-secret", + succeededKeys, + }; + } + writtenSecrets.push(secret); + succeededKeys.push(pair.key); + } + + const updateResult = await api.updateFunction({ + code: params.code, + id: params.id, + ...(params.sourceMap !== undefined ? { sourceMap: params.sourceMap } : {}), + }); + if (updateResult.error) { + return { + kind: "error", + payload: extractErrorPayload(updateResult.error), + stage: "redeploy", + succeededKeys, + }; + } + const redeployed = updateResult.data?.data; + if (!redeployed) { + return { + kind: "error", + payload: { + code: "client_error", + message: "Redeploy returned no data", + }, + stage: "redeploy", + succeededKeys, + }; + } + + return { + kind: "ok", + result: { + redeploy: redeployed, + ...(writtenSecrets.length > 0 ? { secrets: writtenSecrets } : {}), + }, + }; +} class FunctionsRedeployCommand extends Command { static description = @@ -27,13 +171,21 @@ class FunctionsRedeployCommand extends Command { Use to push a new bundle OR to refresh secret bindings into the running handler. The same file is fine for both: the deploy reads the bindings table fresh on every call, so passing the existing - bundle picks up any secret writes since the last deploy.`; + bundle picks up any secret writes since the last deploy. + + Pass --secret KEY=VALUE (repeatable) to write secrets BEFORE the + redeploy fires; one update-function call then refreshes every new + binding. Keys must match \`^[A-Z_][A-Z0-9_]*$\` (uppercase letters, + digits, underscores; first character is a letter or underscore). + With one or more --secret flags the redeploy fans out to multiple + API calls (set-secret per pair, then update-function).`; static summary = "Redeploy a function from a bundled handler file"; static examples = [ "<%= config.bin %> functions:redeploy --id --file ./bundle.js", "<%= config.bin %> functions:redeploy --id --file ./bundle.js --source-map-file ./bundle.js.map", + "<%= config.bin %> functions:redeploy --id --file ./bundle.js --secret OPENAI_KEY=sk-... --secret OWNER_EMAIL=me@example.com", ]; static flags = { @@ -67,6 +219,11 @@ class FunctionsRedeployCommand extends Command { description: "Optional path to a source map for the bundle. Used to symbolicate stack traces in the function's logs.", }), + secret: Flags.string({ + description: + "Secret KEY=VALUE to write on the function before the redeploy fires. Repeatable. KEY must match `^[A-Z_][A-Z0-9_]*$`; VALUE may contain `=` (only the first `=` is treated as a delimiter). Passing one or more --secret flags fans out to set-secret per pair then a single update-function call so the new bindings land in the same redeploy.", + multiple: true, + }), time: Flags.boolean({ description: TIME_FLAG_DESCRIPTION, }), @@ -76,6 +233,18 @@ class FunctionsRedeployCommand extends Command { const { flags } = await this.parse(FunctionsRedeployCommand); await runWithTiming(flags.time, async () => { + // Validate --secret pairs BEFORE any disk read or API call so + // a malformed input fails fast with a clear error and zero + // side effects. The fast path (no --secret flags) skips the + // secret-write loop entirely. + const rawSecrets = flags.secret ?? []; + const parsedSecrets = parseSecretFlags(rawSecrets); + if (parsedSecrets.kind === "error") { + process.stderr.write(`${parsedSecrets.message}\n`); + process.exitCode = 1; + return; + } + // Reads inside the timed block: --time captures disk I/O too, // which is the latency the flag is meant to surface. const code = readTextFileFlag(flags.file, "--file"); @@ -111,29 +280,64 @@ class FunctionsRedeployCommand extends Command { configDir: this.config.configDir, }; - const result = await updateFunction({ - path: { id: flags.id }, - body: { - code, - ...(sourceMap !== undefined ? { sourceMap } : {}), - }, - client: apiClient.client, - responseStyle: "fields", + // Adapter: thin wrappers around the generated SDK calls, + // routed through host 1 (apiClient.client). The function + // CRUD and secrets endpoints are not on host 2. + const apiSurface: RedeployApiSurface = { + setSecret: (p) => + setFunctionSecret({ + body: { value: p.value }, + client: apiClient.client, + path: { id: p.id, key: p.key }, + responseStyle: "fields", + }), + updateFunction: (p) => + updateFunction({ + body: { + code: p.code, + ...(p.sourceMap !== undefined ? { sourceMap: p.sourceMap } : {}), + }, + client: apiClient.client, + path: { id: p.id }, + responseStyle: "fields", + }), + }; + + const outcome = await runRedeployWithSecrets(apiSurface, { + code, + id: flags.id, + secrets: parsedSecrets.secrets, + ...(sourceMap !== undefined ? { sourceMap } : {}), }); - if (result.error) { - const errorPayload = extractErrorPayload(result.error); - writeErrorWithHints(errorPayload); + if (outcome.kind === "error") { + if (outcome.stage === "set-secret") { + const succeeded = + outcome.succeededKeys.length > 0 + ? outcome.succeededKeys.join(", ") + : "(none)"; + process.stderr.write( + `Writing secret ${outcome.failedKey} failed before the redeploy; succeeded keys so far: ${succeeded}. The new bundle has NOT been deployed. Re-run \`primitive functions:set-secret --id ${flags.id} --key ${outcome.failedKey} --value \` after fixing the cause, then \`primitive functions:redeploy --id ${flags.id} --file \`.\n`, + ); + } else if (outcome.stage === "redeploy") { + const succeeded = + outcome.succeededKeys.length > 0 + ? outcome.succeededKeys.join(", ") + : "(none)"; + process.stderr.write( + `Secrets [${succeeded}] were written, but the redeploy step failed; the new bindings are NOT yet live. Re-run \`primitive functions:redeploy --id ${flags.id} --file \` once the cause is fixed.\n`, + ); + } + writeErrorWithHints(outcome.payload); removeStaleSavedCredentialOnUnauthorized({ ...authFailureContext, - payload: errorPayload, + payload: outcome.payload, }); process.exitCode = 1; return; } - const envelope = result.data as { data?: FunctionDetail } | undefined; - this.log(JSON.stringify(envelope?.data ?? null, null, 2)); + this.log(JSON.stringify(outcome.result.redeploy, null, 2)); }); } } diff --git a/cli-node/tests/oclif/functions-deploy.test.ts b/cli-node/tests/oclif/functions-deploy.test.ts new file mode 100644 index 0000000..42cf131 --- /dev/null +++ b/cli-node/tests/oclif/functions-deploy.test.ts @@ -0,0 +1,448 @@ +import type { + CreateFunctionResult, + FunctionDetail, + FunctionSecretWriteResult, +} from "@primitivedotdev/sdk/api"; +import { describe, expect, it, vi } from "vitest"; +import { + type DeployApiSurface, + parseSecretFlags, + runDeployWithSecrets, +} from "../../src/oclif/commands/functions-deploy.js"; +import { COMMANDS } from "../../src/oclif/index.js"; + +const FN_ID = "22222222-2222-4222-8222-222222222222"; +const FN_NAME = "test-fn"; +const BUNDLE = + "export default { async fetch(req) { return new Response('ok'); } };"; + +function makeCreateResult( + overrides: Partial = {}, +): CreateFunctionResult { + return { + deploy_status: "deployed", + gateway_url: `https://${FN_ID}.fn.primitive.dev`, + id: FN_ID, + name: FN_NAME, + ...overrides, + }; +} + +function makeFunctionDetail( + overrides: Partial = {}, +): FunctionDetail { + return { + code: BUNDLE, + created_at: "2026-05-09T00:00:00.000Z", + deploy_status: "deployed", + gateway_url: `https://${FN_ID}.fn.primitive.dev`, + id: FN_ID, + name: FN_NAME, + updated_at: "2026-05-09T00:00:00.000Z", + ...overrides, + }; +} + +function makeSecret( + overrides: Partial = {}, +): FunctionSecretWriteResult { + return { + created: true, + created_at: "2026-05-10T00:00:00.000Z", + key: "API_TOKEN", + updated_at: "2026-05-10T00:00:00.000Z", + ...overrides, + }; +} + +// Build a fake API surface that records every call so a test can +// assert how runDeployWithSecrets routed its inputs. By default +// every method resolves with a successful envelope; individual tests +// override one method to simulate failures, and the override is +// invoked after the call is recorded (so call counts stay accurate +// regardless of which response shape the override returns). +function makeApi(overrides: Partial = {}): { + api: DeployApiSurface; + calls: { + createFunction: { name: string; code: string; sourceMap?: string }[]; + setSecret: { id: string; key: string; value: string }[]; + updateFunction: { id: string; code: string; sourceMap?: string }[]; + }; +} { + const calls = { + createFunction: [] as { + name: string; + code: string; + sourceMap?: string; + }[], + setSecret: [] as { id: string; key: string; value: string }[], + updateFunction: [] as { + id: string; + code: string; + sourceMap?: string; + }[], + }; + + const api: DeployApiSurface = { + createFunction: vi.fn( + async (params: { name: string; code: string; sourceMap?: string }) => { + calls.createFunction.push(params); + if (overrides.createFunction) return overrides.createFunction(params); + return { + data: { + data: makeCreateResult({ name: params.name }), + }, + }; + }, + ), + setSecret: vi.fn( + async (params: { id: string; key: string; value: string }) => { + calls.setSecret.push(params); + if (overrides.setSecret) return overrides.setSecret(params); + return { data: { data: makeSecret({ key: params.key }) } }; + }, + ), + updateFunction: vi.fn( + async (params: { id: string; code: string; sourceMap?: string }) => { + calls.updateFunction.push(params); + if (overrides.updateFunction) return overrides.updateFunction(params); + return { data: { data: makeFunctionDetail({ code: params.code }) } }; + }, + ), + }; + + return { api, calls }; +} + +describe("functions:deploy command", () => { + it("registers in the COMMANDS map", () => { + expect(COMMANDS["functions:deploy"]).toBeDefined(); + }); +}); + +describe("parseSecretFlags", () => { + it("returns an empty list for an empty input", () => { + const result = parseSecretFlags([]); + expect(result.kind).toBe("ok"); + if (result.kind === "ok") { + expect(result.secrets).toEqual([]); + } + }); + + it("parses a single KEY=VALUE pair", () => { + const result = parseSecretFlags(["API_TOKEN=abc123"]); + expect(result.kind).toBe("ok"); + if (result.kind === "ok") { + expect(result.secrets).toEqual([{ key: "API_TOKEN", value: "abc123" }]); + } + }); + + it("parses multiple pairs in order", () => { + const result = parseSecretFlags(["FIRST=one", "SECOND=two", "THIRD=three"]); + expect(result.kind).toBe("ok"); + if (result.kind === "ok") { + expect(result.secrets).toEqual([ + { key: "FIRST", value: "one" }, + { key: "SECOND", value: "two" }, + { key: "THIRD", value: "three" }, + ]); + } + }); + + it("only splits on the FIRST '=' so VALUE may contain '='", () => { + const result = parseSecretFlags(["BASE64=YWJjPWRlZg=="]); + expect(result.kind).toBe("ok"); + if (result.kind === "ok") { + expect(result.secrets).toEqual([ + { key: "BASE64", value: "YWJjPWRlZg==" }, + ]); + } + }); + + it("allows an empty VALUE (KEY=)", () => { + const result = parseSecretFlags(["EMPTY="]); + expect(result.kind).toBe("ok"); + if (result.kind === "ok") { + expect(result.secrets).toEqual([{ key: "EMPTY", value: "" }]); + } + }); + + it("rejects an entry with no '='", () => { + const result = parseSecretFlags(["NO_EQUALS"]); + expect(result.kind).toBe("error"); + if (result.kind === "error") { + expect(result.message).toContain("KEY=VALUE"); + } + }); + + it("rejects an entry with an empty KEY", () => { + const result = parseSecretFlags(["=value"]); + expect(result.kind).toBe("error"); + if (result.kind === "error") { + expect(result.message).toContain("KEY"); + } + }); + + it("rejects a lowercase KEY", () => { + const result = parseSecretFlags(["lower_case=value"]); + expect(result.kind).toBe("error"); + if (result.kind === "error") { + expect(result.message).toContain("lower_case"); + } + }); + + it("rejects a KEY starting with a digit", () => { + const result = parseSecretFlags(["1KEY=value"]); + expect(result.kind).toBe("error"); + if (result.kind === "error") { + expect(result.message).toContain("1KEY"); + } + }); + + it("rejects a KEY with hyphens", () => { + const result = parseSecretFlags(["MY-KEY=value"]); + expect(result.kind).toBe("error"); + if (result.kind === "error") { + expect(result.message).toContain("MY-KEY"); + } + }); + + it("accepts underscores and digits after the first character", () => { + const result = parseSecretFlags(["_PRIVATE_KEY_2=val"]); + expect(result.kind).toBe("ok"); + if (result.kind === "ok") { + expect(result.secrets).toEqual([{ key: "_PRIVATE_KEY_2", value: "val" }]); + } + }); + + it("returns the first error and stops scanning", () => { + const result = parseSecretFlags(["GOOD=ok", "bad=nope", "ALSO_BAD"]); + expect(result.kind).toBe("error"); + if (result.kind === "error") { + // Should mention the lowercase key, not the missing-equals one + // (we bail on the first failure to avoid surfacing later noise). + expect(result.message).toContain("bad"); + expect(result.message).not.toContain("ALSO_BAD"); + } + }); +}); + +describe("runDeployWithSecrets (no --secret)", () => { + it("creates the function and skips setSecret + updateFunction", async () => { + const { api, calls } = makeApi(); + + const outcome = await runDeployWithSecrets(api, { + code: BUNDLE, + name: FN_NAME, + secrets: [], + }); + + expect(outcome.kind).toBe("ok"); + if (outcome.kind === "ok") { + expect(outcome.result.created.id).toBe(FN_ID); + expect(outcome.result.redeploy).toBeUndefined(); + expect(outcome.result.secrets).toBeUndefined(); + } + expect(calls.createFunction).toEqual([{ code: BUNDLE, name: FN_NAME }]); + expect(calls.setSecret).toEqual([]); + expect(calls.updateFunction).toEqual([]); + }); + + it("passes sourceMap through to createFunction when provided", async () => { + const { api, calls } = makeApi(); + + await runDeployWithSecrets(api, { + code: BUNDLE, + name: FN_NAME, + secrets: [], + sourceMap: "{}", + }); + + expect(calls.createFunction).toEqual([ + { code: BUNDLE, name: FN_NAME, sourceMap: "{}" }, + ]); + }); + + it("surfaces a create error and never writes secrets or redeploys", async () => { + const { api, calls } = makeApi({ + createFunction: async () => ({ + error: { code: "conflict", message: "name already exists" }, + }), + }); + + const outcome = await runDeployWithSecrets(api, { + code: BUNDLE, + name: FN_NAME, + secrets: [{ key: "API_TOKEN", value: "abc" }], + }); + + expect(outcome.kind).toBe("error"); + if (outcome.kind === "error") { + expect(outcome.stage).toBe("create"); + expect(outcome.payload).toEqual({ + code: "conflict", + message: "name already exists", + }); + } + expect(calls.setSecret).toEqual([]); + expect(calls.updateFunction).toEqual([]); + }); + + it("flags a 2xx-with-empty-data create response as a client error", async () => { + const { api } = makeApi({ + createFunction: async () => ({ data: {} }), + }); + + const outcome = await runDeployWithSecrets(api, { + code: BUNDLE, + name: FN_NAME, + secrets: [], + }); + + expect(outcome.kind).toBe("error"); + if (outcome.kind === "error") { + expect(outcome.stage).toBe("create"); + } + }); +}); + +describe("runDeployWithSecrets (--secret K=V)", () => { + it("creates, writes each secret in order, then redeploys with the same bundle", async () => { + const redeployedDetail = makeFunctionDetail({ + updated_at: "2026-05-10T00:00:01.000Z", + }); + const { api, calls } = makeApi({ + updateFunction: async (p) => { + // The redeploy must reuse the same bundle so the running + // handler picks up the new bindings without changing + // behavior. + expect(p.code).toBe(BUNDLE); + return { data: { data: redeployedDetail } }; + }, + }); + + const outcome = await runDeployWithSecrets(api, { + code: BUNDLE, + name: FN_NAME, + secrets: [ + { key: "FIRST", value: "1" }, + { key: "SECOND", value: "2" }, + ], + }); + + expect(outcome.kind).toBe("ok"); + if (outcome.kind === "ok") { + // Final payload is the redeployed detail, not the create + // result, because that's the state the user actually + // deployed. + expect(outcome.result.redeploy?.updated_at).toBe( + "2026-05-10T00:00:01.000Z", + ); + expect(outcome.result.secrets).toHaveLength(2); + } + expect(calls.createFunction).toHaveLength(1); + expect(calls.setSecret).toEqual([ + { id: FN_ID, key: "FIRST", value: "1" }, + { id: FN_ID, key: "SECOND", value: "2" }, + ]); + expect(calls.updateFunction).toHaveLength(1); + }); + + it("reports a set-secret error after the function was created, with succeeded keys so far", async () => { + const { api, calls } = makeApi({ + setSecret: async (p) => { + if (p.key === "SECOND") { + return { + error: { code: "validation", message: "value too long" }, + }; + } + return { data: { data: makeSecret({ key: p.key }) } }; + }, + }); + + const outcome = await runDeployWithSecrets(api, { + code: BUNDLE, + name: FN_NAME, + secrets: [ + { key: "FIRST", value: "1" }, + { key: "SECOND", value: "2" }, + { key: "THIRD", value: "3" }, + ], + }); + + expect(outcome.kind).toBe("error"); + if (outcome.kind === "error" && outcome.stage === "set-secret") { + expect(outcome.failedKey).toBe("SECOND"); + expect(outcome.succeededKeys).toEqual(["FIRST"]); + expect(outcome.created.id).toBe(FN_ID); + expect(outcome.payload).toEqual({ + code: "validation", + message: "value too long", + }); + } + // The third secret must never be attempted after the second + // failed; updateFunction must not fire. + expect(calls.setSecret.map((c) => c.key)).toEqual(["FIRST", "SECOND"]); + expect(calls.updateFunction).toEqual([]); + }); + + it("reports a redeploy error after every secret was written", async () => { + const { api, calls } = makeApi({ + updateFunction: async () => ({ + error: { code: "server_error", message: "runtime offline" }, + }), + }); + + const outcome = await runDeployWithSecrets(api, { + code: BUNDLE, + name: FN_NAME, + secrets: [ + { key: "FIRST", value: "1" }, + { key: "SECOND", value: "2" }, + ], + }); + + expect(outcome.kind).toBe("error"); + if (outcome.kind === "error" && outcome.stage === "redeploy") { + expect(outcome.succeededKeys).toEqual(["FIRST", "SECOND"]); + expect(outcome.created.id).toBe(FN_ID); + } + expect(calls.setSecret).toHaveLength(2); + expect(calls.updateFunction).toHaveLength(1); + }); + + it("rejects an empty 2xx setSecret response so we don't fake a write success", async () => { + const { api, calls } = makeApi({ + setSecret: async () => ({ data: {} }), + }); + + const outcome = await runDeployWithSecrets(api, { + code: BUNDLE, + name: FN_NAME, + secrets: [{ key: "API_TOKEN", value: "abc" }], + }); + + expect(outcome.kind).toBe("error"); + if (outcome.kind === "error") { + expect(outcome.stage).toBe("set-secret"); + } + expect(calls.updateFunction).toEqual([]); + }); + + it("rejects an empty 2xx updateFunction response so we don't fake a redeploy", async () => { + const { api } = makeApi({ + updateFunction: async () => ({ data: {} }), + }); + + const outcome = await runDeployWithSecrets(api, { + code: BUNDLE, + name: FN_NAME, + secrets: [{ key: "API_TOKEN", value: "abc" }], + }); + + expect(outcome.kind).toBe("error"); + if (outcome.kind === "error") { + expect(outcome.stage).toBe("redeploy"); + } + }); +}); diff --git a/cli-node/tests/oclif/functions-redeploy.test.ts b/cli-node/tests/oclif/functions-redeploy.test.ts new file mode 100644 index 0000000..f197d82 --- /dev/null +++ b/cli-node/tests/oclif/functions-redeploy.test.ts @@ -0,0 +1,270 @@ +import type { + FunctionDetail, + FunctionSecretWriteResult, +} from "@primitivedotdev/sdk/api"; +import { describe, expect, it, vi } from "vitest"; +import { + type RedeployApiSurface, + runRedeployWithSecrets, +} from "../../src/oclif/commands/functions-redeploy.js"; +import { COMMANDS } from "../../src/oclif/index.js"; + +const FN_ID = "33333333-3333-4333-8333-333333333333"; +const FN_NAME = "test-fn"; +const BUNDLE = + "export default { async fetch(req) { return new Response('ok'); } };"; + +function makeFunctionDetail( + overrides: Partial = {}, +): FunctionDetail { + return { + code: BUNDLE, + created_at: "2026-05-09T00:00:00.000Z", + deploy_status: "deployed", + gateway_url: `https://${FN_ID}.fn.primitive.dev`, + id: FN_ID, + name: FN_NAME, + updated_at: "2026-05-09T00:00:00.000Z", + ...overrides, + }; +} + +function makeSecret( + overrides: Partial = {}, +): FunctionSecretWriteResult { + return { + created: true, + created_at: "2026-05-10T00:00:00.000Z", + key: "API_TOKEN", + updated_at: "2026-05-10T00:00:00.000Z", + ...overrides, + }; +} + +// Build a fake API surface that records every call. Same shape as +// the functions-deploy fake but without createFunction since the +// function already exists in the redeploy flow. +function makeApi(overrides: Partial = {}): { + api: RedeployApiSurface; + calls: { + setSecret: { id: string; key: string; value: string }[]; + updateFunction: { id: string; code: string; sourceMap?: string }[]; + }; +} { + const calls = { + setSecret: [] as { id: string; key: string; value: string }[], + updateFunction: [] as { + id: string; + code: string; + sourceMap?: string; + }[], + }; + + const api: RedeployApiSurface = { + setSecret: vi.fn( + async (params: { id: string; key: string; value: string }) => { + calls.setSecret.push(params); + if (overrides.setSecret) return overrides.setSecret(params); + return { data: { data: makeSecret({ key: params.key }) } }; + }, + ), + updateFunction: vi.fn( + async (params: { id: string; code: string; sourceMap?: string }) => { + calls.updateFunction.push(params); + if (overrides.updateFunction) return overrides.updateFunction(params); + return { data: { data: makeFunctionDetail({ code: params.code }) } }; + }, + ), + }; + + return { api, calls }; +} + +describe("functions:redeploy command", () => { + it("registers in the COMMANDS map", () => { + expect(COMMANDS["functions:redeploy"]).toBeDefined(); + }); +}); + +describe("runRedeployWithSecrets (no --secret)", () => { + it("calls updateFunction once and skips setSecret entirely", async () => { + const { api, calls } = makeApi(); + + const outcome = await runRedeployWithSecrets(api, { + code: BUNDLE, + id: FN_ID, + secrets: [], + }); + + expect(outcome.kind).toBe("ok"); + if (outcome.kind === "ok") { + expect(outcome.result.redeploy.id).toBe(FN_ID); + expect(outcome.result.secrets).toBeUndefined(); + } + expect(calls.setSecret).toEqual([]); + expect(calls.updateFunction).toEqual([{ code: BUNDLE, id: FN_ID }]); + }); + + it("passes sourceMap through to updateFunction when provided", async () => { + const { api, calls } = makeApi(); + + await runRedeployWithSecrets(api, { + code: BUNDLE, + id: FN_ID, + secrets: [], + sourceMap: "{}", + }); + + expect(calls.updateFunction).toEqual([ + { code: BUNDLE, id: FN_ID, sourceMap: "{}" }, + ]); + }); + + it("surfaces an updateFunction error", async () => { + const { api, calls } = makeApi({ + updateFunction: async () => ({ + error: { code: "not_found", message: "function not found" }, + }), + }); + + const outcome = await runRedeployWithSecrets(api, { + code: BUNDLE, + id: FN_ID, + secrets: [], + }); + + expect(outcome.kind).toBe("error"); + if (outcome.kind === "error" && outcome.stage === "redeploy") { + expect(outcome.succeededKeys).toEqual([]); + expect(outcome.payload).toEqual({ + code: "not_found", + message: "function not found", + }); + } + expect(calls.setSecret).toEqual([]); + expect(calls.updateFunction).toHaveLength(1); + }); + + it("flags a 2xx-with-empty-data update response as a client error", async () => { + const { api } = makeApi({ + updateFunction: async () => ({ data: {} }), + }); + + const outcome = await runRedeployWithSecrets(api, { + code: BUNDLE, + id: FN_ID, + secrets: [], + }); + + expect(outcome.kind).toBe("error"); + if (outcome.kind === "error") { + expect(outcome.stage).toBe("redeploy"); + } + }); +}); + +describe("runRedeployWithSecrets (--secret K=V)", () => { + it("writes every secret BEFORE the updateFunction call", async () => { + const { api, calls } = makeApi(); + + const outcome = await runRedeployWithSecrets(api, { + code: BUNDLE, + id: FN_ID, + secrets: [ + { key: "FIRST", value: "1" }, + { key: "SECOND", value: "2" }, + ], + }); + + expect(outcome.kind).toBe("ok"); + if (outcome.kind === "ok") { + expect(outcome.result.redeploy.id).toBe(FN_ID); + expect(outcome.result.secrets).toHaveLength(2); + } + expect(calls.setSecret).toEqual([ + { id: FN_ID, key: "FIRST", value: "1" }, + { id: FN_ID, key: "SECOND", value: "2" }, + ]); + expect(calls.updateFunction).toHaveLength(1); + // Sanity: updateFunction received the same bundle the caller + // provided. The redeploy must not pull a different code body. + expect(calls.updateFunction[0]?.code).toBe(BUNDLE); + }); + + it("reports a set-secret error with succeeded keys so far and skips the redeploy", async () => { + const { api, calls } = makeApi({ + setSecret: async (p) => { + if (p.key === "SECOND") { + return { + error: { code: "validation", message: "value too long" }, + }; + } + return { data: { data: makeSecret({ key: p.key }) } }; + }, + }); + + const outcome = await runRedeployWithSecrets(api, { + code: BUNDLE, + id: FN_ID, + secrets: [ + { key: "FIRST", value: "1" }, + { key: "SECOND", value: "2" }, + { key: "THIRD", value: "3" }, + ], + }); + + expect(outcome.kind).toBe("error"); + if (outcome.kind === "error" && outcome.stage === "set-secret") { + expect(outcome.failedKey).toBe("SECOND"); + expect(outcome.succeededKeys).toEqual(["FIRST"]); + expect(outcome.payload).toEqual({ + code: "validation", + message: "value too long", + }); + } + expect(calls.setSecret.map((c) => c.key)).toEqual(["FIRST", "SECOND"]); + expect(calls.updateFunction).toEqual([]); + }); + + it("reports a redeploy error after every secret was written", async () => { + const { api, calls } = makeApi({ + updateFunction: async () => ({ + error: { code: "server_error", message: "runtime offline" }, + }), + }); + + const outcome = await runRedeployWithSecrets(api, { + code: BUNDLE, + id: FN_ID, + secrets: [ + { key: "FIRST", value: "1" }, + { key: "SECOND", value: "2" }, + ], + }); + + expect(outcome.kind).toBe("error"); + if (outcome.kind === "error" && outcome.stage === "redeploy") { + expect(outcome.succeededKeys).toEqual(["FIRST", "SECOND"]); + } + expect(calls.setSecret).toHaveLength(2); + expect(calls.updateFunction).toHaveLength(1); + }); + + it("rejects an empty 2xx setSecret response so we don't fake a write success", async () => { + const { api, calls } = makeApi({ + setSecret: async () => ({ data: {} }), + }); + + const outcome = await runRedeployWithSecrets(api, { + code: BUNDLE, + id: FN_ID, + secrets: [{ key: "API_TOKEN", value: "abc" }], + }); + + expect(outcome.kind).toBe("error"); + if (outcome.kind === "error") { + expect(outcome.stage).toBe("set-secret"); + } + expect(calls.updateFunction).toEqual([]); + }); +}); From c35c3f319215696cac8b08250078f7efe7ed30cd Mon Sep 17 00:00:00 2001 From: Ethan Byrd Date: Mon, 11 May 2026 10:33:55 -0700 Subject: [PATCH 2/2] Move parseSecretFlags to its own module, reject dups, document shell exposure --- .../src/oclif/commands/functions-deploy.ts | 57 ++------------ .../src/oclif/commands/functions-redeploy.ts | 9 ++- cli-node/src/oclif/secret-flags.ts | 74 +++++++++++++++++++ cli-node/tests/oclif/functions-deploy.test.ts | 22 +++++- 4 files changed, 107 insertions(+), 55 deletions(-) create mode 100644 cli-node/src/oclif/secret-flags.ts diff --git a/cli-node/src/oclif/commands/functions-deploy.ts b/cli-node/src/oclif/commands/functions-deploy.ts index f4e0bf3..9adc10d 100644 --- a/cli-node/src/oclif/commands/functions-deploy.ts +++ b/cli-node/src/oclif/commands/functions-deploy.ts @@ -20,6 +20,11 @@ import { } from "../api-command.js"; import { resolveCliAuth } from "../auth.js"; import { emitRawSendMailFetchWarning } from "../lint/raw-send-mail-fetch.js"; +import { + parseSecretFlags, + SECRET_FLAG_SECURITY_NOTE, + type SecretFlagPair, +} from "../secret-flags.js"; // `primitive functions:deploy` is the agent-grade shortcut for // `functions:create-function`. The underlying operation takes `code` @@ -46,55 +51,6 @@ import { emitRawSendMailFetchWarning } from "../lint/raw-send-mail-fetch.js"; // is a single create call; with one or more --secret flags the // flow fans out to (create + N set-secret + redeploy) API calls. -// Server-side constraint on secret keys. Mirrored client-side so -// malformed input is rejected before any side-effecting API call. -const SECRET_KEY_RE = /^[A-Z_][A-Z0-9_]*$/; - -// Parsed --secret K=V pair. Exported so the unit test can build -// the same value the command produces internally. -export type SecretFlagPair = { key: string; value: string }; - -// Result of parsing the raw oclif --secret strings. Discriminated -// so the caller can decide whether to write a stderr error before -// touching the API surface. -export type ParseSecretFlagsResult = - | { kind: "ok"; secrets: SecretFlagPair[] } - | { kind: "error"; message: string }; - -// Split each `--secret KEY=VALUE` on the FIRST `=`. KEY must match -// `^[A-Z_][A-Z0-9_]*$`; VALUE may contain `=` (only the first one -// is treated as a delimiter). Returns a structured error rather -// than throwing so the caller can write the message to stderr in -// the same envelope shape used elsewhere. -export function parseSecretFlags(raw: string[]): ParseSecretFlagsResult { - const secrets: SecretFlagPair[] = []; - for (const entry of raw) { - const eq = entry.indexOf("="); - if (eq === -1) { - return { - kind: "error", - message: `--secret expects KEY=VALUE (got ${JSON.stringify(entry)}). Example: --secret API_TOKEN=abc123`, - }; - } - const key = entry.slice(0, eq); - const value = entry.slice(eq + 1); - if (key.length === 0) { - return { - kind: "error", - message: `--secret is missing a KEY before '=' (got ${JSON.stringify(entry)}). Example: --secret API_TOKEN=abc123`, - }; - } - if (!SECRET_KEY_RE.test(key)) { - return { - kind: "error", - message: `--secret KEY ${JSON.stringify(key)} does not match ${SECRET_KEY_RE.source} (uppercase letters, digits, underscores; first character is a letter or underscore).`, - }; - } - secrets.push({ key, value }); - } - return { kind: "ok", secrets }; -} - // Final payload runDeployWithSecrets produces on the happy path. // `created` is the initial create result; `redeploy` is the // updateFunction return value after secrets were written. When @@ -343,8 +299,7 @@ class FunctionsDeployCommand extends Command { "Optional path to a source map for the bundle. Stored only on the runtime side and used to symbolicate stack traces.", }), secret: Flags.string({ - description: - "Secret KEY=VALUE to seed on the deployed function. Repeatable. KEY must match `^[A-Z_][A-Z0-9_]*$`; VALUE may contain `=` (only the first `=` is treated as a delimiter). Passing one or more --secret flags fans out the deploy to create-function, set-secret per pair, then a final redeploy so the running handler picks up the bindings.", + description: `Secret KEY=VALUE to seed on the deployed function. Repeatable. KEY must match \`^[A-Z_][A-Z0-9_]*$\`; VALUE may contain \`=\` (only the first \`=\` is treated as a delimiter). Each KEY may only appear once per command. Passing one or more --secret flags fans out the deploy to create-function, set-secret per pair, then a final redeploy so the running handler picks up the bindings. ${SECRET_FLAG_SECURITY_NOTE}`, multiple: true, }), time: Flags.boolean({ diff --git a/cli-node/src/oclif/commands/functions-redeploy.ts b/cli-node/src/oclif/commands/functions-redeploy.ts index f4d0982..e8efcef 100644 --- a/cli-node/src/oclif/commands/functions-redeploy.ts +++ b/cli-node/src/oclif/commands/functions-redeploy.ts @@ -18,7 +18,11 @@ import { } from "../api-command.js"; import { resolveCliAuth } from "../auth.js"; import { emitRawSendMailFetchWarning } from "../lint/raw-send-mail-fetch.js"; -import { parseSecretFlags, type SecretFlagPair } from "./functions-deploy.js"; +import { + parseSecretFlags, + SECRET_FLAG_SECURITY_NOTE, + type SecretFlagPair, +} from "../secret-flags.js"; // `primitive functions:redeploy` is the agent-grade shortcut for // `functions:update-function`. Same file-reading ergonomic as @@ -220,8 +224,7 @@ class FunctionsRedeployCommand extends Command { "Optional path to a source map for the bundle. Used to symbolicate stack traces in the function's logs.", }), secret: Flags.string({ - description: - "Secret KEY=VALUE to write on the function before the redeploy fires. Repeatable. KEY must match `^[A-Z_][A-Z0-9_]*$`; VALUE may contain `=` (only the first `=` is treated as a delimiter). Passing one or more --secret flags fans out to set-secret per pair then a single update-function call so the new bindings land in the same redeploy.", + description: `Secret KEY=VALUE to write on the function before the redeploy fires. Repeatable. KEY must match \`^[A-Z_][A-Z0-9_]*$\`; VALUE may contain \`=\` (only the first \`=\` is treated as a delimiter). Each KEY may only appear once per command. Passing one or more --secret flags fans out to set-secret per pair then a single update-function call so the new bindings land in the same redeploy. ${SECRET_FLAG_SECURITY_NOTE}`, multiple: true, }), time: Flags.boolean({ diff --git a/cli-node/src/oclif/secret-flags.ts b/cli-node/src/oclif/secret-flags.ts new file mode 100644 index 0000000..0c6c1e2 --- /dev/null +++ b/cli-node/src/oclif/secret-flags.ts @@ -0,0 +1,74 @@ +// Shared parsing for the `--secret KEY=VALUE` flag used by both +// functions:deploy and functions:redeploy. Lives in its own module so +// neither command implicitly depends on the other's file path. + +// Server-side constraint on secret keys. Mirrored client-side so +// malformed input is rejected before any side-effecting API call. +export const SECRET_KEY_RE = /^[A-Z_][A-Z0-9_]*$/; + +// Parsed --secret K=V pair. Exported so unit tests can build the +// same value the commands produce internally. +export type SecretFlagPair = { key: string; value: string }; + +// Result of parsing the raw oclif --secret strings. Discriminated +// so the caller can decide whether to write a stderr error before +// touching the API surface. +export type ParseSecretFlagsResult = + | { kind: "ok"; secrets: SecretFlagPair[] } + | { kind: "error"; message: string }; + +// Split each `--secret KEY=VALUE` on the FIRST `=`. KEY must match +// `^[A-Z_][A-Z0-9_]*$`; VALUE may contain `=` (only the first one +// is treated as a delimiter). Duplicate KEYs are rejected: silently +// accepting two pairs with the same key would fan out to two +// setFunctionSecret writes where only the second wins, which is +// almost always a typo and never the intent. +export function parseSecretFlags(raw: string[]): ParseSecretFlagsResult { + const secrets: SecretFlagPair[] = []; + const seenKeys = new Set(); + for (const entry of raw) { + const eq = entry.indexOf("="); + if (eq === -1) { + return { + kind: "error", + message: `--secret expects KEY=VALUE (got ${JSON.stringify(entry)}). Example: --secret API_TOKEN=abc123`, + }; + } + const key = entry.slice(0, eq); + const value = entry.slice(eq + 1); + if (key.length === 0) { + return { + kind: "error", + message: `--secret is missing a KEY before '=' (got ${JSON.stringify(entry)}). Example: --secret API_TOKEN=abc123`, + }; + } + if (!SECRET_KEY_RE.test(key)) { + return { + kind: "error", + message: `--secret KEY ${JSON.stringify(key)} does not match ${SECRET_KEY_RE.source} (uppercase letters, digits, underscores; first character is a letter or underscore).`, + }; + } + if (seenKeys.has(key)) { + return { + kind: "error", + message: `--secret KEY ${JSON.stringify(key)} was passed more than once. Each key may only appear once per command.`, + }; + } + seenKeys.add(key); + secrets.push({ key, value }); + } + return { kind: "ok", secrets }; +} + +// Shared flag-description copy so both functions:deploy and +// functions:redeploy advertise the same security caveat and KEY +// constraints. The shell-history note is the load-bearing piece: +// CLI flag values land in ~/.bash_history, `ps aux`, and +// /proc/[pid]/cmdline, so callers handling sensitive values +// should set them via a shell variable (ideally read via `read -s` +// or piped from a secrets manager) and reference the variable on +// the command line. The variable still appears in `ps`-visible +// argv, but at least the literal value does not get archived in +// the user's shell history. +export const SECRET_FLAG_SECURITY_NOTE = + 'Note: values passed on the command line are visible in shell history (e.g. ~/.bash_history) and to other users via `ps aux` / /proc/[pid]/cmdline. For sensitive values prefer `--secret KEY="$VAR"` where `$VAR` is set out-of-band (read -s, a secrets manager, etc.).'; diff --git a/cli-node/tests/oclif/functions-deploy.test.ts b/cli-node/tests/oclif/functions-deploy.test.ts index 42cf131..1d9ba29 100644 --- a/cli-node/tests/oclif/functions-deploy.test.ts +++ b/cli-node/tests/oclif/functions-deploy.test.ts @@ -6,10 +6,10 @@ import type { import { describe, expect, it, vi } from "vitest"; import { type DeployApiSurface, - parseSecretFlags, runDeployWithSecrets, } from "../../src/oclif/commands/functions-deploy.js"; import { COMMANDS } from "../../src/oclif/index.js"; +import { parseSecretFlags } from "../../src/oclif/secret-flags.js"; const FN_ID = "22222222-2222-4222-8222-222222222222"; const FN_NAME = "test-fn"; @@ -225,6 +225,26 @@ describe("parseSecretFlags", () => { expect(result.message).not.toContain("ALSO_BAD"); } }); + + it("rejects duplicate keys before any API call would fire", () => { + // Silently accepting two pairs with the same key fans out to two + // setFunctionSecret writes where only the second wins. That is + // almost always a typo, not the intent, so we error up front. + const result = parseSecretFlags(["API_TOKEN=first", "API_TOKEN=second"]); + expect(result.kind).toBe("error"); + if (result.kind === "error") { + expect(result.message).toContain("API_TOKEN"); + expect(result.message).toContain("more than once"); + } + }); + + it("rejects duplicate keys even when other keys precede the dup", () => { + const result = parseSecretFlags(["FIRST=one", "SECOND=two", "FIRST=three"]); + expect(result.kind).toBe("error"); + if (result.kind === "error") { + expect(result.message).toContain("FIRST"); + } + }); }); describe("runDeployWithSecrets (no --secret)", () => {