From a7ed089dc89b330354fb50905616f73237c5013c Mon Sep 17 00:00:00 2001 From: noahim Date: Thu, 13 Aug 2026 13:29:44 +0900 Subject: [PATCH] feat(secrets): support prompt secret references --- AGENTS.md | 2 + docs/providers/provider-runtimes.md | 22 +++ electron/main/browser/secret-service.ts | 48 ++++-- electron/main/browser/secret-vault.ts | 63 +++++++ electron/providers/claude-sdk-runtime.ts | 36 +++- .../providers/codex-app-server-runtime.ts | 33 +++- .../layout/settings-dialog-secrets.tsx | 18 +- src/lib/secrets/secret-references.ts | 154 ++++++++++++++++++ src/lib/secrets/secrets.ts | 7 +- tests/codex-app-server-mcp-lifecycle.test.ts | 62 ++++++- tests/secrets-settings.test.tsx | 4 + tests/secrets.test.ts | 80 +++++++++ 12 files changed, 492 insertions(+), 37 deletions(-) create mode 100644 src/lib/secrets/secret-references.ts diff --git a/AGENTS.md b/AGENTS.md index 92684f0e..e3896f2b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -117,6 +117,8 @@ Rules: - Inject secrets for the **primary user turn only** — never for introspection, aux, or secondary read-only analysis queries. - Claude injects at the `options.env` layer (kept out of `buildClaudeDiagnostics`); Codex injects shell variables via per-thread `shell_environment_policy.set.` overrides, forwarded on **both** `thread/start` and `thread/resume`. Secret-bound primary Codex turns use a disposable App Server process with the same environment so `bearer_token_env_var` MCP authentication works without exposing values to shared clients. - Never write a secret value to `console.*`, a `BridgeEvent`, a transcript, or the thread key. Log only counts, env-var names, and skip reasons. +- Parse `@secret:{ENV_VAR_NAME}` only from the current primary input. Resolve the key through the main-owned vault and add only value-free availability guidance to provider prompt text; never substitute plaintext into the prompt. +- Missing, malformed, over-limit, and reserved prompt references must stay unavailable. Never satisfy an unresolved reference from the ambient process environment. - This is an *automatic-leak* guarantee, not a sandbox: a deliberate `echo $NAME` can still surface a bound value. Keep the Settings > Secrets copy honest about this. ## Terminal Surface Guardrails diff --git a/docs/providers/provider-runtimes.md b/docs/providers/provider-runtimes.md index b56ec0e7..1d16bfd8 100644 --- a/docs/providers/provider-runtimes.md +++ b/docs/providers/provider-runtimes.md @@ -15,6 +15,28 @@ decisions are recorded in The renderer submits a selected provider and model with each turn. `electron/main/ipc/provider.ts` validates the request, forwards it into the dedicated desktop `host-service` child process, and `electron/providers/runtime.ts` dispatches to the matching provider runtime. +## Prompt secret references + +A primary task prompt may reference an injectable Settings → Secrets entry by +its environment-variable name: `@secret:{OPENAI_API_KEY}`. The current input is +parsed inside the provider runtime, and only the non-secret key is used to ask +the main-owned vault for the matching value. The value follows the existing +bound-secret environment path: Claude receives it through `options.env`, while +Codex uses a disposable App Server process plus per-thread +`shell_environment_policy.set.*` overrides. It never replaces the token with +plaintext or adds the value to renderer state, provider prompt text, events, +transcripts, diagnostics, or logs. + +The provider prompt receives value-free guidance that maps a resolved reference +to `$OPENAI_API_KEY`, so shell commands and supported MCP authentication can use +it without the model seeing the value. Missing, malformed, over-limit, and +reserved references are described as unavailable and are never satisfied from +an ambient process variable. Reference resolution applies only to the current +primary input; secondary read-only execution does not receive referenced or +manually bound secrets. A command that deliberately prints the environment +variable can still surface its value, so this remains an automatic-leak +boundary rather than a sandbox. + ## On-demand Advisor consults `Settings → Providers → Advisor` arms an isolated read-only Advisor that the diff --git a/electron/main/browser/secret-service.ts b/electron/main/browser/secret-service.ts index 6ea0215e..5886ca34 100644 --- a/electron/main/browser/secret-service.ts +++ b/electron/main/browser/secret-service.ts @@ -48,9 +48,10 @@ export async function revealSecret(id: string) { } /** - * Resolve a task's bound secret ids to an environment map for provider shell - * commands and supported MCP authentication. MAIN-PROCESS ONLY: this returns - * plaintext values, so it must never reach preload or the renderer. + * Resolve a task's bound secret ids and explicit prompt reference keys to an + * environment map for provider shell commands and supported MCP + * authentication. MAIN-PROCESS ONLY: this returns plaintext values, so it must + * never reach preload, the renderer, model-visible text, or logs. * * Returns an empty map on any resolution failure rather than throwing, so a * misconfigured or unavailable vault degrades to "no injected secrets" instead @@ -59,30 +60,47 @@ export async function revealSecret(id: string) { */ export async function resolveBoundSecretEnv(args: { ids: readonly string[]; + referenceKeys?: readonly string[]; }): Promise> { const ids = args.ids.filter( (id): id is string => typeof id === "string" && id.length > 0, ); - if (ids.length === 0) { + const referenceKeys = (args.referenceKeys ?? []).filter( + (key): key is string => typeof key === "string" && key.trim().length > 0, + ); + if (ids.length === 0 && referenceKeys.length === 0) { return {}; } try { - const { env, skipped } = await getVault().resolveEnvForIds(ids); + const secretVault = getVault(); + const [boundResult, referenceResult] = await Promise.all([ + ids.length > 0 + ? secretVault.resolveEnvForIds(ids) + : Promise.resolve({ env: {}, skipped: [] }), + referenceKeys.length > 0 + ? secretVault.resolveEnvForReferences(referenceKeys) + : Promise.resolve({ env: {}, skipped: [] }), + ]); + const env = { ...boundResult.env, ...referenceResult.env }; + const skippedLabels = [ + ...boundResult.skipped.map((entry) => + entry.envVarName + ? `${entry.envVarName}:${entry.reason}` + : entry.reason, + ), + ...referenceResult.skipped.map((entry) => + entry.key ? `${entry.key}:${entry.reason}` : entry.reason, + ), + ]; const injectedNames = Object.keys(env); - if (skipped.length > 0 || injectedNames.length > 0) { + if (skippedLabels.length > 0 || injectedNames.length > 0) { console.info( - `[secrets] resolved ${injectedNames.length}/${ids.length} bound secret(s) for injection` + + `[secrets] resolved ${injectedNames.length}/${ids.length + referenceKeys.length} secret binding/reference(s) for injection` + (injectedNames.length > 0 ? ` (${injectedNames.join(", ")})` : "") + - (skipped.length > 0 - ? `; skipped ${skipped - .map((entry) => - entry.envVarName - ? `${entry.envVarName}:${entry.reason}` - : entry.reason, - ) - .join(", ")}` + (skippedLabels.length > 0 + ? `; skipped ${skippedLabels.join(", ")}` : ""), ); } diff --git a/electron/main/browser/secret-vault.ts b/electron/main/browser/secret-vault.ts index 981ca6ad..72268603 100644 --- a/electron/main/browser/secret-vault.ts +++ b/electron/main/browser/secret-vault.ts @@ -6,6 +6,7 @@ import { buildSecretPreview, ENV_VAR_NAME_MAX_LENGTH, ENV_VAR_NAME_PATTERN, + MAX_BOUND_SECRETS, isReservedEnvVarName, normalizeEnvVarName, normalizeSecretName, @@ -256,6 +257,68 @@ export class SecretVault { return { env, skipped }; } + /** + * Resolve `@secret:{ENV_VAR_NAME}` keys without exposing vault metadata or + * plaintext outside the main-owned provider runtime path. + * + * Validation and the reserved-key denylist are repeated here even though the + * prompt parser filters candidates first. This is the security boundary: a + * direct or future caller must not be able to claim PATH, CODEX_HOME, or any + * other Stave/runtime-owned variable. + */ + async resolveEnvForReferences(keys: readonly string[]): Promise<{ + env: Record; + skipped: Array<{ key: string; reason: string }>; + }> { + const env: Record = {}; + const skipped: Array<{ key: string; reason: string }> = []; + const uniqueKeys = [ + ...new Set( + keys + .filter((key): key is string => typeof key === "string") + .map((key) => key.trim()), + ), + ]; + if (uniqueKeys.length === 0) { + return { env, skipped }; + } + + this.assertSecureEncryption(); + const document = await this.readDocument(); + for (const [index, key] of uniqueKeys.entries()) { + if (index >= MAX_BOUND_SECRETS) { + skipped.push({ key, reason: "limit-exceeded" }); + continue; + } + if ( + key.length === 0 || + key.length > ENV_VAR_NAME_MAX_LENGTH || + !ENV_VAR_NAME_PATTERN.test(key) + ) { + skipped.push({ key: "", reason: "invalid-name" }); + continue; + } + if (isReservedEnvVarName(key)) { + skipped.push({ key, reason: "reserved-name" }); + continue; + } + + const matches = document.secrets.filter( + (entry) => entry.envVarName === key, + ); + if (matches.length === 0) { + skipped.push({ key, reason: "not-found" }); + continue; + } + if (matches.length > 1) { + skipped.push({ key, reason: "duplicate-name" }); + continue; + } + env[key] = this.decryptValue(matches[0]!); + } + return { env, skipped }; + } + private decryptValue(entry: StoredSecret): string { try { return this.args.crypto.decryptString( diff --git a/electron/providers/claude-sdk-runtime.ts b/electron/providers/claude-sdk-runtime.ts index b4d9de23..2d97e873 100644 --- a/electron/providers/claude-sdk-runtime.ts +++ b/electron/providers/claude-sdk-runtime.ts @@ -7,6 +7,7 @@ import type { import { buildProviderTurnPrompt, filterPromptRetrievedContext, + getProviderNativeSlashCommandInput, resolveProviderResumeSessionId, } from "../../src/lib/providers/provider-request-translators"; import { @@ -86,6 +87,11 @@ import { toClaudeSdkMcpServerConfig, } from "../main/stave-local-mcp-manifest"; import { resolveBoundSecretEnv } from "../main/browser/secret-service"; +import { MAX_BOUND_SECRETS } from "../../src/lib/secrets/secrets"; +import { + appendPromptSecretReferenceContext, + parsePromptSecretReferences, +} from "../../src/lib/secrets/secret-references"; import { parseBooleanEnv, parsePositiveIntEnv, @@ -4793,15 +4799,22 @@ export async function streamClaudeWithSdk( } const secondaryReadOnly = args.executionPolicy === "secondary-read-only"; - // Resolve bound-secret env for the primary user turn only. Secondary - // read-only analysis turns never receive injected secrets. + const boundSecretIds = args.runtimeOptions?.boundSecretIds ?? []; + const promptSecretReferences = parsePromptSecretReferences({ + prompt: args.prompt, + maxResolvableReferences: + MAX_BOUND_SECRETS - new Set(boundSecretIds).size, + }); + // Resolve bound and explicitly referenced secrets for the primary user + // turn only. Secondary read-only analysis turns never receive them. const boundSecretEnv = secondaryReadOnly || - !args.runtimeOptions?.boundSecretIds || - args.runtimeOptions.boundSecretIds.length === 0 + (boundSecretIds.length === 0 && + promptSecretReferences.resolutionKeys.length === 0) ? {} : await resolveBoundSecretEnv({ - ids: args.runtimeOptions.boundSecretIds, + ids: boundSecretIds, + referenceKeys: promptSecretReferences.resolutionKeys, }); const existingSessionId = secondaryReadOnly ? undefined @@ -4880,12 +4893,23 @@ export async function streamClaudeWithSdk( : ["stave:current-task-awareness"], }) : args.conversation; - const providerPrompt = buildProviderTurnPrompt({ + const nativeSlashCommandInput = promptConversation + ? getProviderNativeSlashCommandInput(promptConversation) + : null; + const providerPromptBase = buildProviderTurnPrompt({ providerId: args.providerId, prompt: args.prompt, conversation: promptConversation, activeResumeSessionId: existingSessionId ?? null, }); + const providerPrompt = nativeSlashCommandInput + ? providerPromptBase + : appendPromptSecretReferenceContext({ + prompt: providerPromptBase, + parsed: promptSecretReferences, + availableEnvNames: Object.keys(boundSecretEnv), + disabledForSecondaryReadOnly: secondaryReadOnly, + }); const activatedSkillSlugs = collectClaudeActivatedSkillSlugs({ conversation: args.conversation, }); diff --git a/electron/providers/codex-app-server-runtime.ts b/electron/providers/codex-app-server-runtime.ts index d68c6545..da8fac1f 100644 --- a/electron/providers/codex-app-server-runtime.ts +++ b/electron/providers/codex-app-server-runtime.ts @@ -69,6 +69,11 @@ import { import { getCodexMcpRegistrationStatus } from "../main/codex-mcp"; import { readPrimaryStaveLocalMcpManifest } from "../main/stave-local-mcp-manifest"; import { resolveBoundSecretEnv } from "../main/browser/secret-service"; +import { MAX_BOUND_SECRETS } from "../../src/lib/secrets/secrets"; +import { + appendPromptSecretReferenceContext, + parsePromptSecretReferences, +} from "../../src/lib/secrets/secret-references"; import { buildCodexInstructionProfileKey, resolveCodexWorkerProfile } from "./codex-runtime-config"; import { buildWorkerExecutionMetadata, type WorkerExecutionMetadata } from "../../src/lib/providers/worker-mode"; import { @@ -2303,14 +2308,24 @@ export async function streamCodexWithAppServer( const workerExecution = workerProfile ? buildWorkerExecutionMetadata(workerProfile) : null; const codexCapabilities = getCodexVersionCapabilities(codexExecutablePath); + const boundSecretIds = runtimeOptions?.boundSecretIds ?? []; + const promptSecretReferences = parsePromptSecretReferences({ + prompt: args.prompt, + maxResolvableReferences: + MAX_BOUND_SECRETS - new Set(boundSecretIds).size, + }); // A per-turn process lets Codex resolve MCP bearer_token_env_var settings - // without exposing bound values to shared clients or read-only analysis. + // without exposing bound or explicitly referenced values to shared clients, + // model-visible text, or read-only analysis. const boundSecretEnv = secondaryReadOnly || - !runtimeOptions?.boundSecretIds || - runtimeOptions.boundSecretIds.length === 0 + (boundSecretIds.length === 0 && + promptSecretReferences.resolutionKeys.length === 0) ? {} - : await resolveBoundSecretEnv({ ids: runtimeOptions.boundSecretIds }); + : await resolveBoundSecretEnv({ + ids: boundSecretIds, + referenceKeys: promptSecretReferences.resolutionKeys, + }); const codexRuntimeEnv = buildCodexCliEnv({ executablePath: codexExecutablePath, }); @@ -2519,7 +2534,7 @@ export async function streamCodexWithAppServer( ? false : await hasConnectedStaveLocalMcpForCodex(); - const providerPrompt = + const providerPromptBase = nativeSlashCommandInput ?? buildProviderTurnPrompt({ providerId: args.providerId, @@ -2534,6 +2549,14 @@ export async function streamCodexWithAppServer( }) : args.conversation, }); + const providerPrompt = nativeSlashCommandInput + ? providerPromptBase + : appendPromptSecretReferenceContext({ + prompt: providerPromptBase, + parsed: promptSecretReferences, + availableEnvNames: Object.keys(boundSecretEnv), + disabledForSecondaryReadOnly: secondaryReadOnly, + }); const goalCommandEvents = await runCodexGoalSlashCommand({ client, diff --git a/src/components/layout/settings-dialog-secrets.tsx b/src/components/layout/settings-dialog-secrets.tsx index a1010fcc..e37d36ef 100644 --- a/src/components/layout/settings-dialog-secrets.tsx +++ b/src/components/layout/settings-dialog-secrets.tsx @@ -223,7 +223,7 @@ export function SecretsSettingsCard() { <>

A secret's value is never shown to an agent. Give a secret an - environment variable name to bind it to a task from the composer — - its value is then available to that task's shell and supported MCP - authentication (e.g. $OPENAI_API_KEY) without entering - the model's context. A command that echoes the variable can still + environment variable name, then reference it in a prompt as + @secret:{"{OPENAI_API_KEY}"} + or bind it from the composer. Its value is available to that + turn's shell and supported MCP authentication as + $OPENAI_API_KEY without entering the + model's context. A command that echoes the variable can still surface it.

@@ -316,7 +318,11 @@ export function SecretsSettingsCard() { onChange={(event) => setEnvVarName(event.target.value)} /> - Set this to let a task inject the value into its runtime as + Set this to reference the secret in a prompt as + + @secret:{`{${envVarName.trim() || "NAME"}}`} + + or bind it from the composer. The value is injected as ${envVarName.trim() || "NAME"} diff --git a/src/lib/secrets/secret-references.ts b/src/lib/secrets/secret-references.ts new file mode 100644 index 00000000..ad0ef44e --- /dev/null +++ b/src/lib/secrets/secret-references.ts @@ -0,0 +1,154 @@ +import { + ENV_VAR_NAME_MAX_LENGTH, + ENV_VAR_NAME_PATTERN, + MAX_BOUND_SECRETS, + isReservedEnvVarName, +} from "./secrets"; + +const SECRET_REFERENCE_PATTERN = /@secret:\{([^{}\r\n]*)\}/g; +const MAX_PARSED_SECRET_REFERENCES = MAX_BOUND_SECRETS * 2; + +export type PromptSecretReferenceStatus = + | "candidate" + | "invalid" + | "protected" + | "limit-exceeded"; + +export interface PromptSecretReference { + /** A validated environment-variable key, or an empty string when invalid. */ + key: string; + status: PromptSecretReferenceStatus; +} + +export interface ParsedPromptSecretReferences { + references: PromptSecretReference[]; + /** Valid, non-reserved keys that the main-process vault may resolve. */ + resolutionKeys: string[]; + /** Extra unique reference tokens omitted from the bounded result. */ + overflowCount: number; +} + +function normalizeResolvableLimit(value: number | undefined) { + if (value === undefined || !Number.isFinite(value)) { + return MAX_BOUND_SECRETS; + } + return Math.max(0, Math.min(MAX_BOUND_SECRETS, Math.floor(value))); +} + +/** + * Parse secret references from the current user input without touching the + * vault. Reference keys are environment-variable names, not secret values. + * + * The result is bounded so a prompt cannot cause unbounded vault lookups or + * prompt guidance. Invalid and protected keys are retained only as safe + * statuses; an invalid raw key is never copied into generated prompt text. + */ +export function parsePromptSecretReferences(args: { + prompt: string; + maxResolvableReferences?: number; +}): ParsedPromptSecretReferences { + const references: PromptSecretReference[] = []; + const resolutionKeys: string[] = []; + const seenKeys = new Set(); + let invalidReferenceSeen = false; + let overflowCount = 0; + const resolvableLimit = normalizeResolvableLimit( + args.maxResolvableReferences, + ); + + for (const match of args.prompt.matchAll(SECRET_REFERENCE_PATTERN)) { + const rawKey = match[1] ?? ""; + const key = rawKey.trim(); + const validKey = + key.length > 0 && + key.length <= ENV_VAR_NAME_MAX_LENGTH && + ENV_VAR_NAME_PATTERN.test(key); + + if (!validKey) { + // One generic invalid entry is enough to inform the model without + // reflecting arbitrary prompt text back into generated instructions. + if (!invalidReferenceSeen) { + if (references.length >= MAX_PARSED_SECRET_REFERENCES) { + overflowCount += 1; + } else { + references.push({ key: "", status: "invalid" }); + invalidReferenceSeen = true; + } + } + continue; + } + if (seenKeys.has(key)) { + continue; + } + seenKeys.add(key); + + if (references.length >= MAX_PARSED_SECRET_REFERENCES) { + overflowCount += 1; + continue; + } + if (isReservedEnvVarName(key)) { + references.push({ key, status: "protected" }); + continue; + } + if (resolutionKeys.length >= resolvableLimit) { + references.push({ key, status: "limit-exceeded" }); + continue; + } + references.push({ key, status: "candidate" }); + resolutionKeys.push(key); + } + + return { references, resolutionKeys, overflowCount }; +} + +/** + * Append value-free runtime guidance to the provider prompt. The model sees + * only reference/env-var names and availability; plaintext values stay in the + * provider process environment. + */ +export function appendPromptSecretReferenceContext(args: { + prompt: string; + parsed: ParsedPromptSecretReferences; + availableEnvNames: readonly string[]; + disabledForSecondaryReadOnly?: boolean; +}): string { + if ( + args.parsed.references.length === 0 && + args.parsed.overflowCount === 0 + ) { + return args.prompt; + } + + const availableEnvNames = new Set(args.availableEnvNames); + const lines = args.parsed.references.map((reference) => { + if (reference.status === "invalid") { + return "- An invalid @secret reference was ignored. Keys must be POSIX environment-variable names."; + } + if (reference.status === "protected") { + return `- @secret:{${reference.key}} was refused because ${reference.key} is a protected runtime variable.`; + } + if (reference.status === "limit-exceeded") { + return `- @secret:{${reference.key}} was not injected because this turn reached the ${MAX_BOUND_SECRETS}-secret limit.`; + } + if (args.disabledForSecondaryReadOnly) { + return `- @secret:{${reference.key}} is unavailable in a secondary read-only turn.`; + } + if (!availableEnvNames.has(reference.key)) { + return `- @secret:{${reference.key}} is unavailable because Settings > Secrets has no injectable secret using ${reference.key}.`; + } + return `- @secret:{${reference.key}} is available to shell commands and supported MCP authentication as $${reference.key}.`; + }); + + if (args.parsed.overflowCount > 0) { + lines.push( + `- ${args.parsed.overflowCount} additional secret reference(s) were ignored because the prompt reference list is bounded.`, + ); + } + + return [ + args.prompt, + "[Stave Secret References]", + "Secret values are not included in this prompt. Use available variables without printing or echoing their values. Do not fall back to ambient process variables for unavailable or refused references.", + ...lines, + ].join("\n\n"); +} diff --git a/src/lib/secrets/secrets.ts b/src/lib/secrets/secrets.ts index df695514..21198fc0 100644 --- a/src/lib/secrets/secrets.ts +++ b/src/lib/secrets/secrets.ts @@ -1,8 +1,9 @@ /** * Shared types for the general secret store (API tokens and other secret - * values). Unlike Lens saved accounts, secrets are not bound to a hostname and - * are never auto-filled anywhere; they are stored encrypted at rest and only - * revealed on explicit user request. + * values). Unlike Lens saved accounts, secrets are not bound to a hostname. + * They are stored encrypted at rest and only revealed on explicit user + * request, or injected into a primary provider runtime after an explicit task + * binding or `@secret:{ENV_VAR_NAME}` prompt reference. */ /** Metadata safe to send to the renderer. Never carries the secret value. */ diff --git a/tests/codex-app-server-mcp-lifecycle.test.ts b/tests/codex-app-server-mcp-lifecycle.test.ts index 895c6a9c..d11f76b7 100644 --- a/tests/codex-app-server-mcp-lifecycle.test.ts +++ b/tests/codex-app-server-mcp-lifecycle.test.ts @@ -250,12 +250,20 @@ class FakeChild extends EventEmitter { let nextScenario: FakeScenario = "full-lifecycle"; let nextBoundSecretEnv: Record = {}; let resolvedSecretRequestCount = 0; +const resolvedSecretRequests: Array<{ + ids: readonly string[]; + referenceKeys?: readonly string[]; +}> = []; const fakeChildren: FakeChild[] = []; const tempDirectories: string[] = []; mock.module("../electron/main/browser/secret-service", () => ({ - resolveBoundSecretEnv: async () => { + resolveBoundSecretEnv: async (args: { + ids: readonly string[]; + referenceKeys?: readonly string[]; + }) => { resolvedSecretRequestCount += 1; + resolvedSecretRequests.push(args); return { ...nextBoundSecretEnv }; }, })); @@ -277,6 +285,7 @@ afterEach(async () => { nextScenario = "full-lifecycle"; nextBoundSecretEnv = {}; resolvedSecretRequestCount = 0; + resolvedSecretRequests.length = 0; fakeChildren.length = 0; mock.restore(); await Promise.all( @@ -475,7 +484,8 @@ describe("Codex App Server MCP lifecycle mapping", () => { providerId: "codex", taskId: "secondary:execution-1", executionPolicy: "secondary-read-only", - prompt: "Inspect the runtime", + prompt: + "Inspect the runtime with @secret:{STAVE_TEST_BOUND_MCP_TOKEN}", cwd: process.cwd(), runtimeOptions: { codexBinaryPath: "/tmp/fake-codex-secondary", @@ -532,6 +542,12 @@ describe("Codex App Server MCP lifecycle mapping", () => { networkAccess: false, }, }); + expect(JSON.stringify(turnStart?.params ?? {})).toContain( + "STAVE_TEST_BOUND_MCP_TOKEN} is unavailable in a secondary read-only turn", + ); + expect(JSON.stringify(turnStart?.params ?? {})).not.toContain( + "must-not-reach-secondary", + ); expect( child.receivedMessages.some( (message) => message.method === "thread/delete", @@ -583,6 +599,48 @@ describe("Codex App Server MCP lifecycle mapping", () => { expect(fakeChildren[0]?.killed).toBe(false); }); + test("resolves prompt references without putting secret values in model-visible text", async () => { + const envName = "STAVE_TEST_REFERENCED_TOKEN"; + const secretValue = "reference-only-secret-value"; + nextScenario = "completed-only"; + nextBoundSecretEnv = { [envName]: secretValue }; + const runtime = await import( + `../electron/providers/codex-app-server-runtime?secret-reference-test=${Date.now()}-${Math.random()}` + ); + + const events = await runtime.streamCodexWithAppServer({ + providerId: "codex", + taskId: "task-secret-reference", + prompt: `Use @secret:{${envName}}, report @secret:{MISSING_TOKEN}, and refuse @secret:{PATH}.`, + cwd: process.cwd(), + runtimeOptions: { + codexBinaryPath: "/tmp/fake-codex-secret-reference", + }, + }); + + expect(resolvedSecretRequestCount).toBe(1); + expect(resolvedSecretRequests).toEqual([ + { + ids: [], + referenceKeys: [envName, "MISSING_TOKEN"], + }, + ]); + expect(fakeChildren).toHaveLength(2); + expect(fakeChildren[1]?.spawnEnv[envName]).toBe(secretValue); + + const turnStart = fakeChildren[1]?.receivedMessages.find( + (message) => message.method === "turn/start", + ); + const modelVisibleText = JSON.stringify(turnStart?.params ?? {}); + expect(modelVisibleText).toContain(`$${envName}`); + expect(modelVisibleText).toContain( + "@secret:{MISSING_TOKEN} is unavailable", + ); + expect(modelVisibleText).toContain("PATH is a protected runtime variable"); + expect(modelVisibleText).not.toContain(secretValue); + expect(JSON.stringify(events)).not.toContain(secretValue); + }); + test("restarts App Server when project MCP config appears between turns", async () => { const cwd = await mkdtemp( path.join(tmpdir(), "stave-codex-project-refresh-"), diff --git a/tests/secrets-settings.test.tsx b/tests/secrets-settings.test.tsx index 32bdfd1a..2e794df7 100644 --- a/tests/secrets-settings.test.tsx +++ b/tests/secrets-settings.test.tsx @@ -10,8 +10,12 @@ describe("SecretsSettingsCard", () => { expect(html).toContain("Secrets"); expect(html).toContain("Add secret"); expect(html).toContain("Store API tokens and other secret values"); + expect(html).toContain( + "Assign an environment variable name, then reference it in a prompt as @secret:{NAME}", + ); expect(html).toContain("Loading secrets"); expect(html).toContain("value is never shown to an agent"); + expect(html).toContain("@secret:{OPENAI_API_KEY}"); expect(html).toContain("supported MCP authentication"); expect(html).not.toContain("plain-secret-value"); }); diff --git a/tests/secrets.test.ts b/tests/secrets.test.ts index 36b346f3..2177c742 100644 --- a/tests/secrets.test.ts +++ b/tests/secrets.test.ts @@ -14,6 +14,10 @@ import { normalizeEnvVarName, normalizeSecretName, } from "../src/lib/secrets/secrets"; +import { + appendPromptSecretReferenceContext, + parsePromptSecretReferences, +} from "../src/lib/secrets/secret-references"; const tempDirs: string[] = []; @@ -336,3 +340,79 @@ describe("SecretVault.resolveEnvForIds", () => { }); }); }); + +describe("prompt secret references", () => { + test("parses valid keys and classifies malformed and protected keys", () => { + const parsed = parsePromptSecretReferences({ + prompt: + "Use @secret:{OPENAI_API_KEY}, retry @secret:{MISSING_KEY}, refuse @secret:{PATH}, and ignore @secret:{bad-name}. @secret:{OPENAI_API_KEY}", + }); + + expect(parsed.resolutionKeys).toEqual([ + "OPENAI_API_KEY", + "MISSING_KEY", + ]); + expect(parsed.references).toEqual([ + { key: "OPENAI_API_KEY", status: "candidate" }, + { key: "MISSING_KEY", status: "candidate" }, + { key: "PATH", status: "protected" }, + { key: "", status: "invalid" }, + ]); + }); + + test("adds only value-free availability guidance to the provider prompt", () => { + const secretValue = "must-never-enter-provider-prompt"; + const parsed = parsePromptSecretReferences({ + prompt: + "Authenticate with @secret:{OPENAI_API_KEY}; also try @secret:{MISSING_KEY} and @secret:{CODEX_HOME}.", + }); + const providerPrompt = appendPromptSecretReferenceContext({ + prompt: + "Authenticate with @secret:{OPENAI_API_KEY}; also try @secret:{MISSING_KEY} and @secret:{CODEX_HOME}.", + parsed, + availableEnvNames: ["OPENAI_API_KEY"], + }); + + expect(providerPrompt).toContain("$OPENAI_API_KEY"); + expect(providerPrompt).toContain( + "@secret:{MISSING_KEY} is unavailable", + ); + expect(providerPrompt).toContain( + "CODEX_HOME is a protected runtime variable", + ); + expect(providerPrompt).toContain("Secret values are not included"); + expect(providerPrompt).not.toContain(secretValue); + }); +}); + +describe("SecretVault.resolveEnvForReferences", () => { + test("resolves valid keys and skips missing and protected references", async () => { + const { vault } = createHarness(); + await vault.upsert({ + name: "OpenAI", + value: "sk-reference-value", + envVarName: "OPENAI_API_KEY", + }); + + const result = await vault.resolveEnvForReferences([ + "OPENAI_API_KEY", + "MISSING_KEY", + "PATH", + ]); + + expect(result.env).toEqual({ OPENAI_API_KEY: "sk-reference-value" }); + expect(result.skipped).toEqual([ + { key: "MISSING_KEY", reason: "not-found" }, + { key: "PATH", reason: "reserved-name" }, + ]); + }); + + test("rejects malformed reference keys at the vault boundary", async () => { + const { vault } = createHarness(); + + expect(await vault.resolveEnvForReferences(["bad-name"])).toEqual({ + env: {}, + skipped: [{ key: "", reason: "invalid-name" }], + }); + }); +});