diff --git a/apps/mobile/src/features/threads/SecretRequestCard.tsx b/apps/mobile/src/features/threads/SecretRequestCard.tsx index 7b67287fb9..8a68199e00 100644 --- a/apps/mobile/src/features/threads/SecretRequestCard.tsx +++ b/apps/mobile/src/features/threads/SecretRequestCard.tsx @@ -2,8 +2,10 @@ import { SECRET_REQUEST_DEFAULT_PLACEHOLDER, SECRET_REQUEST_PRIVACY_NOTE, secretRequestAnswerInput, + secretRequestComplete, secretRequestDisplay, secretRequestFailureMessage, + secretRequestFields, type SecretRequestItem, } from "@t3tools/client-runtime/secret-request"; import { @@ -68,14 +70,18 @@ function PendingSecretRequestForm(props: { reportFailure: false, reportDefect: false, }); - const [secret, setSecret] = useState(""); + const fields = secretRequestFields(item); + const [values, setValues] = useState>(() => new Map()); + const complete = secretRequestComplete(item, values); const [submitting, setSubmitting] = useState(false); const [error, setError] = useState(null); // Submit then a tap can both run before a re-render; this guard is synchronous. const inFlight = useRef(false); const send = async ( - reply: { readonly type: "save"; readonly secret: string } | { readonly type: "decline" }, + reply: + | { readonly type: "save"; readonly values: ReadonlyMap } + | { readonly type: "decline" }, ) => { const input = secretRequestAnswerInput(item, reply); if (input === null || inFlight.current) return; @@ -87,7 +93,7 @@ function PendingSecretRequestForm(props: { setSubmitting(false); }); if (result._tag === "Success") { - setSecret(""); + setValues(new Map()); return; } if (!isAtomCommandInterrupted(result)) { @@ -103,22 +109,32 @@ function PendingSecretRequestForm(props: { {item.reason} ) : null} - void send({ type: "save", secret })} - /> + {fields.map((field) => ( + + {/* A single field is titled by the card itself. */} + {item.fields === undefined ? null : ( + {field.label} + )} + + setValues((current) => new Map(current).set(field.name, value)) + } + editable={!submitting} + secureTextEntry + autoCorrect={false} + autoCapitalize="none" + autoComplete="off" + textContentType="none" + importantForAutofill="no" + spellCheck={false} + returnKeyType="done" + onSubmitEditing={() => void send({ type: "save", values })} + /> + + ))} {error !== null ? ( void send({ type: "save", secret })} + disabled={submitting || !complete} + onPress={() => void send({ type: "save", values })} /> diff --git a/apps/server/src/mcp/OrchestratorMcpService.ts b/apps/server/src/mcp/OrchestratorMcpService.ts index 242f6feb09..dbf4406f3c 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.ts @@ -2361,6 +2361,12 @@ const make = Effect.gen(function* () { requestSecret: (scope, input) => Effect.gen(function* () { const { scope: threadScope, parent } = yield* loadThreadCaller(scope, "request_secret"); + if (input.fields !== undefined && input.placeholder !== undefined) { + return yield* failure( + "invalid_request", + "placeholder is for single-field requests; give each entry in fields its own.", + ); + } const threadId = threadScope.thread.threadId; const run = ThreadManagementService.latestActiveRun(parent); if ( @@ -2395,6 +2401,7 @@ const make = Effect.gen(function* () { label: input.label, reason: input.reason, ...(input.placeholder === undefined ? {} : { placeholder: input.placeholder }), + ...(input.fields === undefined ? {} : { fields: input.fields }), secretStatus, }) .pipe( @@ -2473,14 +2480,16 @@ const make = Effect.gen(function* () { yield* Effect.annotateCurrentSpan({ "secret_request.status": status }); yield* Metrics.increment(Metrics.secretRequestsTotal, { status }); if (status !== "saved") return { status }; - const secretRef = yield* secretRequests.savedRef({ threadId: threadId, turnItemId }); - if (Option.isNone(secretRef)) { + // Saved means the files were written before the card said so; none + // left means a storage fault, or the agent already used and deleted them. + const saved = yield* secretRequests.saved({ threadId: threadId, turnItemId }); + if (Option.isNone(saved)) { return yield* failure( "orchestration_error", "The user saved the secret, but it could not be read. Ask again with a new clientRequestId.", ); } - return { status, secretRef: secretRef.value }; + return { status, ...saved.value }; }).pipe(Effect.withSpan("OrchestratorMcpService.requestSecret")), capabilities: (scope) => diff --git a/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts b/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts index cb0bb2d53d..6ac2f4cecc 100644 --- a/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts +++ b/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts @@ -1136,6 +1136,11 @@ describe("orchestrator MCP toolkit", () => { SecretRequests.layer.pipe( Layer.provide(layerMemorySecretStore), Layer.provide(layerOrchestration), + Layer.provide( + ServerConfig.layerTest(process.cwd(), { prefix: "t3-mcp-secret-requests-" }).pipe( + Layer.provide(NodeServices.layer), + ), + ), ), ), Layer.provide( @@ -2057,6 +2062,7 @@ describe("orchestrator MCP toolkit", () => { expect(secretCall.isError).toBe(false); const secretResult = secretCall.structuredContent as { status: string; + files: Record; secretRef?: string; }; expect(secretResult.status).toBe("saved"); @@ -2064,9 +2070,19 @@ describe("orchestrator MCP toolkit", () => { (yield* orchestrator.getThreadShell(parentThreadId))?.pendingRuntimeRequest ?? null, ).toBeNull(); expect(secretResult.secretRef).toMatch(/^secret-ref:[0-9a-f]{32}$/); + // The agent gets the value only as a file it reads by path. + const secretFileSystem = yield* FileSystem.FileSystem.pipe( + Effect.provide(NodeServices.layer), + ); + expect(Object.keys(secretResult.files)).toEqual(["secret"]); + expect(yield* secretFileSystem.readFileString(secretResult.files.secret!)).toBe( + "github-webhook-secret", + ); + // The value appears nowhere in what the agent received or the thread recorded. const received = [ ...secretCall.content.map((part) => ("text" in part ? part.text : "")), - ...Object.values(secretResult), + JSON.stringify(secretResult), + JSON.stringify(yield* orchestrator.getThreadProjection(parentThreadId)), ]; expect(received.some((value) => value.includes("github-webhook-secret"))).toBe(false); @@ -2093,6 +2109,63 @@ describe("orchestrator MCP toolkit", () => { .pipe(Effect.flip); expect(reused.message).toContain("already used"); + // Several values on one card: one file per field, and no ref. + const pairFiber = yield* invoke("request_secret", { + label: "Cloudflare API credentials", + reason: "Stored in OpenBao for the deploy job.", + fields: [ + { name: "account_id", label: "Account ID" }, + { name: "api_token", label: "API token", placeholder: "Paste the token" }, + ], + clientRequestId: "cloudflare-credentials", + }).pipe(Effect.forkChild); + const pairCard = yield* Effect.gen(function* () { + while (true) { + const projection = yield* orchestrator.getThreadProjection(parentThreadId); + const found = projection.turnItems.find( + (item) => + item.type === "secret_request" && + item.secretStatus === "pending" && + item.label === "Cloudflare API credentials", + ); + if (found?.type === "secret_request") return found; + yield* Effect.sleep("5 millis"); + } + }); + expect(pairCard.fields?.map((field) => field.name)).toEqual([ + "account_id", + "api_token", + ]); + yield* secretRequests.answer({ + threadId: parentThreadId, + turnItemId: pairCard.id, + answer: { + type: "save", + secrets: { account_id: "acct-77", api_token: "cf-token-88" }, + }, + }); + const pairCall = yield* Fiber.join(pairFiber); + expect(pairCall.isError).toBe(false); + const pairResult = pairCall.structuredContent as { + status: string; + files: Record; + secretRef?: string; + }; + expect(pairResult.status).toBe("saved"); + expect(pairResult.secretRef).toBeUndefined(); + expect(yield* secretFileSystem.readFileString(pairResult.files.account_id!)).toBe( + "acct-77", + ); + expect(yield* secretFileSystem.readFileString(pairResult.files.api_token!)).toBe( + "cf-token-88", + ); + const pairReceived = [ + ...pairCall.content.map((part) => ("text" in part ? part.text : "")), + JSON.stringify(pairResult), + JSON.stringify(yield* orchestrator.getThreadProjection(parentThreadId)), + ].join("\n"); + expect(pairReceived).not.toMatch(/acct-77|cf-token-88/); + const delegatedCall = yield* invoke("delegate_task", { task: delegatedPrompt, target: { @@ -4310,6 +4383,11 @@ describe("orchestrator MCP toolkit", () => { SecretRequests.layer.pipe( Layer.provide(layerMemorySecretStore), Layer.provide(layerOrchestration), + Layer.provide( + ServerConfig.layerTest(process.cwd(), { prefix: "t3-mcp-secret-requests-" }).pipe( + Layer.provide(NodeServices.layer), + ), + ), ), ), Layer.provide( diff --git a/apps/server/src/mcp/toolkits/orchestrator/tools.ts b/apps/server/src/mcp/toolkits/orchestrator/tools.ts index 0378519d61..e899dfd6ec 100644 --- a/apps/server/src/mcp/toolkits/orchestrator/tools.ts +++ b/apps/server/src/mcp/toolkits/orchestrator/tools.ts @@ -174,7 +174,7 @@ const DeleteScheduledTaskTool = Tool.make("delete_scheduled_task", { const RequestSecretTool = Tool.make("request_secret", { description: - "Ask the user for a secret (a token, API key, signing secret, password) through a private card in this thread, and wait for them to answer. The value is kept by the app and NEVER returned to you or shown in the transcript. When saved, the result carries a secretRef: pass it to a tool that accepts one (e.g. schedule_task's signature.secretRef). It works once. Never ask for secrets in chat, and never invent one.", + "Ask the user for ANY secret (a token, API key, password, login, signing secret) through a private card in this thread, and wait for them to answer. Use fields to ask for several values on one card, e.g. an ID and a key. The value is never in the result or the transcript: when saved, files maps each field name ('secret' without fields) to a private file on this environment holding exactly that value. Use it without printing it or reading it into your reply, e.g. TOKEN=\"$(cat )\" cmd, bao kv put =- < , or rbw add (reads stdin), then delete the files with rm. Unused files expire after 24 hours. A single-field request also returns a secretRef for a tool that accepts one (schedule_task's signature.secretRef); it works once. Never ask for secrets in chat, and never invent one.", parameters: McpRouting.parameters(OrchestratorMcpRequestSecretInput), success: OrchestratorMcpRequestSecretResult, failure: OrchestratorMcpFailure, diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index d87a17c892..8cd868d3bb 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -8786,6 +8786,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio label: command.label, reason: command.reason, ...(command.placeholder === undefined ? {} : { placeholder: command.placeholder }), + ...(command.fields === undefined ? {} : { fields: command.fields }), secretStatus: command.secretStatus, }; yield* emit( diff --git a/apps/server/src/secrets/SecretRequests.test.ts b/apps/server/src/secrets/SecretRequests.test.ts index b751b0b205..1d13c13fc3 100644 --- a/apps/server/src/secrets/SecretRequests.test.ts +++ b/apps/server/src/secrets/SecretRequests.test.ts @@ -9,6 +9,7 @@ import { } from "@t3tools/contracts"; import * as Clock from "effect/Clock"; import * as Effect from "effect/Effect"; +import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Metric from "effect/Metric"; import * as TestClock from "effect/testing/TestClock"; @@ -24,19 +25,27 @@ import * as SecretRequests from "./SecretRequests.ts"; const threadId = ThreadId.make("thread-orchestrator"); const turnItemId = TurnItemId.make("turn-item:secret-request:1"); const projectId = ProjectId.make("project-1"); +const twoFields = [ + { name: "client_id", label: "Client ID" }, + { name: "client_secret", label: "Client secret", placeholder: "Paste the secret" }, +]; const withService = ( body: (input: { readonly service: SecretRequests.SecretRequests["Service"]; readonly stored: Map; readonly dispatched: Array; - }) => Effect.Effect, + /** Where request files live, under the environment's state directory. */ + readonly filesRoot: string; + }) => Effect.Effect, options: { readonly threadId?: ThreadId; readonly runStatus?: string; readonly removeFails?: boolean; readonly closedFirst?: boolean; readonly failedRecords?: number; + /** The card's fields; absent for a single-field request. */ + readonly fields?: ReadonlyArray<{ name: string; label: string; placeholder?: string }>; } = {}, ) => Effect.gen(function* () { @@ -48,6 +57,9 @@ const withService = ( const layerDependencies = Layer.mergeAll( NodeCrypto.layer, NodeServices.layer, + ServerConfig.layerTest(process.cwd(), { prefix: "t3-secret-request-files-" }).pipe( + Layer.provide(NodeServices.layer), + ), Layer.succeed( ServerSecretStore.ServerSecretStore, ServerSecretStore.ServerSecretStore.of({ @@ -101,6 +113,7 @@ const withService = ( type: "secret_request", label: "GitHub token", reason: "Used as GH_TOKEN.", + ...(options.fields === undefined ? {} : { fields: options.fields }), secretStatus, }, ], @@ -124,8 +137,17 @@ const withService = ( ); return yield* Effect.gen(function* () { const service = yield* SecretRequests.SecretRequests; - return yield* body({ service, stored, dispatched }); - }).pipe(Effect.provide(SecretRequests.layer.pipe(Layer.provide(layerDependencies)))); + const config = yield* ServerConfig.ServerConfig; + return yield* body({ + service, + stored, + dispatched, + filesRoot: `${config.stateDir}/secret-requests`, + }); + }).pipe( + Effect.provide(SecretRequests.layer.pipe(Layer.provideMerge(layerDependencies))), + Effect.scoped, + ); }); const valuesOf = (stored: Map) => @@ -148,7 +170,7 @@ it.effect("a saved answer becomes a one-use ref, and the thread only learns it w }); assert.notInclude(Object.values(dispatched[0] as object).map(String), "ghp_secret"); - const ref = Option.getOrThrow(yield* service.savedRef({ threadId, turnItemId })); + const ref = Option.getOrThrow(yield* service.saved({ threadId, turnItemId })).secretRef!; assert.equal(yield* service.consume({ ref, projectId }), "ghp_secret"); assert.isFalse(valuesOf(stored).some((value) => value.includes("ghp_secret"))); const again = yield* service.consume({ ref, projectId }).pipe(Effect.flip); @@ -165,7 +187,7 @@ it.effect("two concurrent uses of one ref hand the value out once", () => turnItemId, answer: { type: "save", secret: "ghp_secret" }, }); - const ref = Option.getOrThrow(yield* service.savedRef({ threadId, turnItemId })); + const ref = Option.getOrThrow(yield* service.saved({ threadId, turnItemId })).secretRef!; const results = yield* Effect.all( [service.consume({ ref, projectId }), service.consume({ ref, projectId })].map( Effect.result, @@ -204,7 +226,7 @@ it.effect("a save whose record failed can be saved again", () => turnItemId, answer: { type: "save", secret: "ghp_secret" }, }); - const ref = Option.getOrThrow(yield* service.savedRef({ threadId, turnItemId })); + const ref = Option.getOrThrow(yield* service.saved({ threadId, turnItemId })).secretRef!; assert.equal(yield* service.consume({ ref, projectId }), "ghp_secret"); }), { failedRecords: 1 }, @@ -223,7 +245,7 @@ it.effect("a save whose record and cleanup both failed is finished by saving aga turnItemId, answer: { type: "save", secret: "ghp_secret" }, }); - assert.isTrue(Option.isSome(yield* service.savedRef({ threadId, turnItemId }))); + assert.isTrue(Option.isSome(yield* service.saved({ threadId, turnItemId }))); }), { failedRecords: 1, removeFails: true }, ), @@ -237,7 +259,7 @@ it.effect("a ref only works in the project it was entered for", () => turnItemId, answer: { type: "save", secret: "ghp_secret" }, }); - const ref = Option.getOrThrow(yield* service.savedRef({ threadId, turnItemId })); + const ref = Option.getOrThrow(yield* service.saved({ threadId, turnItemId })).secretRef!; const elsewhere = yield* service .consume({ ref, projectId: ProjectId.make("project-other") }) .pipe(Effect.flip); @@ -252,7 +274,7 @@ it.effect("declining stores nothing, and a request is answered once", () => Effect.gen(function* () { yield* service.answer({ threadId, turnItemId, answer: { type: "decline" } }); assert.deepEqual(valuesOf(stored), []); - assert.isTrue(Option.isNone(yield* service.savedRef({ threadId, turnItemId }))); + assert.isTrue(Option.isNone(yield* service.saved({ threadId, turnItemId }))); const late = yield* service .answer({ threadId, turnItemId, answer: { type: "save", secret: "ghp_secret" } }) .pipe(Effect.flip); @@ -291,7 +313,7 @@ it.effect("traces and counts a saved answer without ever recording the value", ( turnItemId, answer: { type: "save", secret: "ghp_secret" }, }); - const ref = Option.getOrThrow(yield* service.savedRef({ threadId, turnItemId })); + const ref = Option.getOrThrow(yield* service.saved({ threadId, turnItemId })).secretRef!; yield* service.consume({ ref, projectId }); }), ).pipe(Effect.withTracer(tracer)); @@ -320,8 +342,8 @@ it.effect("works in threads with long ids, such as a delegated subagent's", () = }); assert.isTrue(Array.from(stored.keys()).every((name) => name.length < 100)); const ref = Option.getOrThrow( - yield* service.savedRef({ threadId: delegatedThreadId, turnItemId }), - ); + yield* service.saved({ threadId: delegatedThreadId, turnItemId }), + ).secretRef!; assert.equal(yield* service.consume({ ref, projectId }), "ghp_secret"); }), { threadId: delegatedThreadId }, @@ -394,10 +416,299 @@ it.effect("a used value that cannot be deleted is not handed out", () => turnItemId, answer: { type: "save", secret: "ghp_secret" }, }); - const ref = Option.getOrThrow(yield* service.savedRef({ threadId, turnItemId })); + const ref = Option.getOrThrow(yield* service.saved({ threadId, turnItemId })).secretRef!; const failed = yield* service.consume({ ref, projectId }).pipe(Effect.flip); assert.include(failed.message, "Could not use"); }), { removeFails: true }, ), ); + +/** Every request directory under the files root, empty when nothing was written. */ +const requestDirs = (filesRoot: string) => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + if (!(yield* fileSystem.exists(filesRoot))) return []; + return yield* fileSystem.readDirectory(filesRoot); + }); + +const modeOf = (path: string) => + FileSystem.FileSystem.use((fileSystem) => fileSystem.stat(path)).pipe( + Effect.map((info) => info.mode & 0o777), + ); + +it.effect("a single-field save also leaves the value in a private file", () => + withService(({ service, filesRoot }) => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + yield* service.answer({ + threadId, + turnItemId, + answer: { type: "save", secret: "ghp_secret" }, + }); + const saved = Option.getOrThrow(yield* service.saved({ threadId, turnItemId })); + const path = saved.files.secret!; + assert.deepEqual(Object.keys(saved.files), ["secret"]); + assert.isTrue(path.startsWith(`${filesRoot}/`)); + assert.equal(yield* fileSystem.readFileString(path), "ghp_secret"); + assert.equal(yield* modeOf(path), 0o600); + assert.isDefined(saved.secretRef); + }), + ), +); + +it.effect("a multi-field save writes each value to its own 0600 file in a 0700 directory", () => + withService( + ({ service, stored, dispatched, filesRoot }) => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + yield* service.answer({ + threadId, + turnItemId, + answer: { type: "save", secrets: { client_id: "id-1", client_secret: "cs-2" } }, + }); + const saved = Option.getOrThrow(yield* service.saved({ threadId, turnItemId })); + assert.deepEqual(Object.keys(saved.files), ["client_id", "client_secret"]); + assert.equal(yield* fileSystem.readFileString(saved.files.client_id!), "id-1"); + assert.equal(yield* fileSystem.readFileString(saved.files.client_secret!), "cs-2"); + assert.equal(yield* modeOf(saved.files.client_id!), 0o600); + assert.equal(yield* modeOf(saved.files.client_secret!), 0o600); + const [dir] = yield* requestDirs(filesRoot); + assert.equal(yield* modeOf(`${filesRoot}/${dir}`), 0o700); + // A ref carries one value, so a multi-field request has none. + assert.isUndefined(saved.secretRef); + assert.deepEqual(valuesOf(stored), []); + // The thread records the fields asked for, never what was typed. + assert.include(dispatched[0] as object, { secretStatus: "saved" }); + assert.notMatch(JSON.stringify(dispatched), /id-1|cs-2/); + }), + { fields: twoFields }, + ), +); + +it.effect("a save missing a field, or with one too many, writes nothing", () => + withService( + ({ service, dispatched, filesRoot }) => + Effect.gen(function* () { + for (const secrets of [ + { client_id: "id-1" }, + { client_id: "id-1", client_secret: "cs-2", extra: "x" }, + ]) { + const error = yield* service + .answer({ threadId, turnItemId, answer: { type: "save", secrets } }) + .pipe(Effect.flip); + assert.equal(error.reason, "fields_mismatch"); + } + const single = yield* service + .answer({ threadId, turnItemId, answer: { type: "save", secret: "x" } }) + .pipe(Effect.flip); + assert.equal(single.reason, "fields_mismatch"); + assert.deepEqual(yield* requestDirs(filesRoot), []); + assert.equal(dispatched.length, 0); + }), + { fields: twoFields }, + ), +); + +it.effect("declining, or losing the save to the card closing, leaves no files", () => + Effect.gen(function* () { + yield* withService(({ service, filesRoot }) => + Effect.gen(function* () { + yield* service.answer({ threadId, turnItemId, answer: { type: "decline" } }); + assert.deepEqual(yield* requestDirs(filesRoot), []); + }), + ); + yield* withService( + ({ service, filesRoot }) => + Effect.gen(function* () { + yield* service + .answer({ + threadId, + turnItemId, + answer: { type: "save", secrets: { client_id: "id-1", client_secret: "cs-2" } }, + }) + .pipe(Effect.flip); + assert.deepEqual(yield* requestDirs(filesRoot), []); + }), + { closedFirst: true, fields: twoFields }, + ); + }), +); + +it.effect("drops request files once they expire, and directories the agent emptied", () => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const config = yield* ServerConfig.ServerConfig; + const root = `${config.stateDir}/secret-requests`; + yield* TestClock.adjust("2 days"); + const now = yield* Clock.currentTimeMillis; + const write = (dir: string, savedAt: number | null) => + Effect.gen(function* () { + yield* fileSystem.makeDirectory(`${root}/${dir}`, { recursive: true }); + if (savedAt === null) return; + yield* fileSystem.writeFileString(`${root}/${dir}/token`, "ghp_secret"); + yield* fileSystem.utimes(`${root}/${dir}/token`, savedAt / 1000, savedAt / 1000); + }); + const stale = "a".repeat(32); + const fresh = "b".repeat(32); + const emptied = "c".repeat(32); + const unrelated = "notes"; + yield* write(stale, now - 25 * 60 * 60 * 1000); + yield* write(fresh, now); + yield* write(emptied, null); + yield* write(unrelated, now - 25 * 60 * 60 * 1000); + + // Building the service runs the first sweep. + yield* Effect.gen(function* () { + yield* SecretRequests.SecretRequests; + }).pipe( + Effect.provide( + SecretRequests.layer.pipe( + Layer.provide(Layer.mock(ThreadManagementService.ThreadManagementService)({})), + ), + ), + Effect.scoped, + ); + + assert.deepEqual((yield* fileSystem.readDirectory(root)).toSorted(), [fresh, unrelated]); + }).pipe( + Effect.provide( + ServerSecretStore.layer.pipe( + Layer.provideMerge( + ServerConfig.layerTest(process.cwd(), { prefix: "t3-secret-request-sweep-" }), + ), + Layer.provideMerge(NodeServices.layer), + ), + ), + ), +); + +it.effect("a value is kept exactly as typed, spaces included", () => + withService(({ service }) => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + yield* service.answer({ + threadId, + turnItemId, + answer: { type: "save", secret: " pass word " }, + }); + const saved = Option.getOrThrow(yield* service.saved({ threadId, turnItemId })); + assert.equal(yield* fileSystem.readFileString(saved.files.secret!), " pass word "); + assert.equal(yield* service.consume({ ref: saved.secretRef!, projectId }), " pass word "); + }), + ), +); + +it.effect("a retry after a failed record replaces a ref that could not be deleted", () => + withService( + ({ service, stored }) => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + yield* service + .answer({ threadId, turnItemId, answer: { type: "save", secret: "first" } }) + .pipe(Effect.flip); + yield* service.answer({ threadId, turnItemId, answer: { type: "save", secret: "second" } }); + const saved = Option.getOrThrow(yield* service.saved({ threadId, turnItemId })); + assert.equal(yield* fileSystem.readFileString(saved.files.secret!), "second"); + assert.deepEqual( + valuesOf(stored).map((value) => JSON.parse(value).value), + ["second"], + ); + }), + { failedRecords: 1, removeFails: true }, + ), +); + +it.effect("a retry after a failed record replaces files that could not be deleted", () => + withService( + ({ service, stored, filesRoot }) => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + yield* service + .answer({ threadId, turnItemId, answer: { type: "save", secret: "first" } }) + .pipe(Effect.flip); + // The failed save's ref was deleted, but its directory was left behind. + const name = Array.from(stored.keys()).find((key) => + /^secret-request-[0-9a-f]{32}$/.test(key), + )!; + stored.delete(name); + const leftover = `${filesRoot}/${name.slice("secret-request-".length)}`; + yield* fileSystem.makeDirectory(leftover, { recursive: true }); + yield* fileSystem.writeFileString(`${leftover}/secret`, "first"); + + yield* service.answer({ threadId, turnItemId, answer: { type: "save", secret: "second" } }); + const saved = Option.getOrThrow(yield* service.saved({ threadId, turnItemId })); + assert.equal(saved.files.secret, `${leftover}/secret`); + assert.equal(yield* fileSystem.readFileString(saved.files.secret!), "second"); + assert.deepEqual( + valuesOf(stored).map((value) => JSON.parse(value).value), + ["second"], + ); + }), + // The store cannot delete, so the failed save's ref survives for the test to find. + { failedRecords: 1, removeFails: true }, + ), +); + +it.effect("an answer whose record failed never deletes what a racing answer saved", () => + withService( + ({ service }) => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const results = yield* Effect.all( + ["value-a", "value-b"].map((secret) => + Effect.result( + service.answer({ threadId, turnItemId, answer: { type: "save", secret } }), + ), + ), + { concurrency: "unbounded" }, + ); + assert.deepEqual(results.map((result) => result._tag).toSorted(), ["Failure", "Success"]); + const saved = Option.getOrThrow(yield* service.saved({ threadId, turnItemId })); + const fromFile = yield* fileSystem.readFileString(saved.files.secret!); + assert.equal(yield* service.consume({ ref: saved.secretRef!, projectId }), fromFile); + }), + { failedRecords: 1 }, + ), +); + +it.effect("a directory the sweep cannot read does not keep it from the rest", () => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const config = yield* ServerConfig.ServerConfig; + const root = `${config.stateDir}/secret-requests`; + yield* TestClock.adjust("2 days"); + const now = yield* Clock.currentTimeMillis; + // Sorted first, and its dangling link fails every stat. + const broken = "0".repeat(32); + const stale = "a".repeat(32); + yield* fileSystem.makeDirectory(`${root}/${broken}`, { recursive: true }); + yield* fileSystem.symlink(`${root}/missing`, `${root}/${broken}/token`); + yield* fileSystem.makeDirectory(`${root}/${stale}`, { recursive: true }); + yield* fileSystem.writeFileString(`${root}/${stale}/token`, "ghp_secret"); + const old = (now - 25 * 60 * 60 * 1000) / 1000; + yield* fileSystem.utimes(`${root}/${stale}/token`, old, old); + + yield* Effect.gen(function* () { + yield* SecretRequests.SecretRequests; + }).pipe( + Effect.provide( + SecretRequests.layer.pipe( + Layer.provide(Layer.mock(ThreadManagementService.ThreadManagementService)({})), + ), + ), + Effect.scoped, + ); + + assert.deepEqual(yield* fileSystem.readDirectory(root), [broken]); + }).pipe( + Effect.provide( + ServerSecretStore.layer.pipe( + Layer.provideMerge( + ServerConfig.layerTest(process.cwd(), { prefix: "t3-secret-request-sweep-" }), + ), + Layer.provideMerge(NodeServices.layer), + ), + ), + ), +); diff --git a/apps/server/src/secrets/SecretRequests.ts b/apps/server/src/secrets/SecretRequests.ts index fadad6ea74..f109888bee 100644 --- a/apps/server/src/secrets/SecretRequests.ts +++ b/apps/server/src/secrets/SecretRequests.ts @@ -1,11 +1,13 @@ /** - * The user's answer goes straight to the server's secret store under a - * one-use SecretRef; orchestration only records the request and its status. - * A tool that needs the value takes the ref and consumes it, so the value - * never reaches the transcript, projections, clients, or model context. + * The user's answer goes straight to private files on the server, one per + * field, that the agent's tools can read by path; a single-field answer is + * also kept under a one-use SecretRef for tools that take one. Orchestration + * only records the request and its status, so the value never reaches the + * transcript, projections, clients, or the tool result. */ import { CommandId, + SECRET_REQUEST_SINGLE_FIELD_NAME, SecretRef, SecretRequestError, type ProjectId, @@ -21,11 +23,13 @@ import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; +import * as Path from "effect/Path"; import * as Schedule from "effect/Schedule"; import * as Schema from "effect/Schema"; import * as Semaphore from "effect/Semaphore"; import * as ServerSecretStore from "../auth/ServerSecretStore.ts"; +import * as ServerConfig from "../config.ts"; import * as Metrics from "../observability/Metrics.ts"; import * as ThreadManagementService from "../orchestration-v2/ThreadManagementService.ts"; @@ -34,6 +38,9 @@ const storeName = (ref: SecretRef) => `secret-request-${ref.slice(SECRET_REF_PRE const REF_PATTERN = /^secret-ref:[0-9a-f]{32}$/; const SECRET_REF_TTL_MS = 24 * 60 * 60 * 1000; const STORE_NAME_PATTERN = /^(secret-request-[0-9a-f]{32})\.bin$/; +/** A request's file directory, or one a save was still writing when it stopped. */ +const FILES_DIR_PATTERN = /^[0-9a-f]{32}(\.tmp-[0-9a-f]+)?$/; +const FIELD_FILE_PATTERN = /^[a-z0-9_-]+$/; const refFor = (salt: string, threadId: ThreadId, turnItemId: string) => SecretRef.make( @@ -49,14 +56,31 @@ const StoredSecret = Schema.fromJsonString( const encodeStored = Schema.encodeEffect(StoredSecret); const decodeStored = Schema.decodeUnknownOption(StoredSecret); +/** What the tool that asked receives for a saved request. Paths, never values. */ +export interface SavedSecret { + /** Absolute path per field name that still has its file. */ + readonly files: Record; + /** Present for a single-field request until a tool consumes it. */ + readonly secretRef?: SecretRef; +} + export class SecretRequests extends Context.Service< SecretRequests, { + /** + * Answers a pending request in a thread. Saving writes every field's + * value to its own file, all or none, before the card says saved. + */ readonly answer: (input: SecretRequestAnswerInput) => Effect.Effect; - readonly savedRef: (input: { + /** The files and ref a saved request left, for the tool that asked. */ + readonly saved: (input: { readonly threadId: ThreadId; readonly turnItemId: string; - }) => Effect.Effect>; + }) => Effect.Effect>; + /** + * Reads and deletes a ref's value. Fails for unknown or used refs, and + * for refs entered in another project. + */ readonly consume: (input: { readonly ref: SecretRef; readonly projectId: ProjectId; @@ -67,12 +91,96 @@ export class SecretRequests extends Context.Service< const make = Effect.gen(function* () { const store = yield* ServerSecretStore.ServerSecretStore; const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const serverConfig = yield* ServerConfig.ServerConfig; const threadManagement = yield* ThreadManagementService.ThreadManagementService; const salt = Buffer.from( yield* store.getOrCreateRandom("secret-request-salt", 32).pipe(Effect.orDie), ).toString("hex"); + // Under the environment's own state, never a project worktree; provider + // CLIs run on this host, so agents read the files by path. + const filesRoot = path.join(serverConfig.stateDir, "secret-requests"); + const filesDir = (ref: SecretRef) => path.join(filesRoot, ref.slice(SECRET_REF_PREFIX.length)); + + /** + * Writes one 0600 file per value into the request's 0700 directory. The + * files appear together by renaming a finished temp directory, so the agent + * never sees half a save. Answers run one at a time, so a directory already + * there is an earlier save of this request whose record failed; it is + * replaced, so the files always hold this save's values. + */ + const writeFiles = (ref: SecretRef, values: Record) => + Effect.gen(function* () { + const target = filesDir(ref); + yield* fileSystem.makeDirectory(filesRoot, { recursive: true, mode: 0o700 }); + yield* fileSystem.chmod(filesRoot, 0o700); + const temp = `${target}.tmp-${NodeCrypto.randomBytes(8).toString("hex")}`; + yield* Effect.gen(function* () { + yield* fileSystem.makeDirectory(temp, { mode: 0o700 }); + yield* fileSystem.chmod(temp, 0o700); + for (const [name, value] of Object.entries(values)) { + const file = path.join(temp, name); + yield* fileSystem.writeFile(file, new TextEncoder().encode(value), { + flag: "wx", + mode: 0o600, + }); + yield* fileSystem.chmod(file, 0o600); + } + yield* fileSystem.remove(target, { recursive: true, force: true }); + yield* fileSystem.rename(temp, target); + }).pipe( + Effect.tapError(() => + fileSystem.remove(temp, { recursive: true, force: true }).pipe(Effect.ignore), + ), + ); + }); + + /** Removes a request's files; a failure is logged, and the sweep retries it. */ + const removeFiles = (ref: SecretRef) => + fileSystem + .remove(filesDir(ref), { recursive: true, force: true }) + .pipe( + Effect.catch((error) => + Effect.logWarning("Could not delete secret request files", { errorTag: error._tag }), + ), + ); + + /** + * Keeps a one-field value under its ref. Set, not create: answers run one at + * a time, so a value already there is an earlier save of this request whose + * record failed, and this save's value replaces it to match the file. + */ + const storeSingle = (ref: SecretRef, projectId: string, value: string) => + Effect.gen(function* () { + const encoded = yield* encodeStored({ + projectId, + value, + savedAt: yield* Clock.currentTimeMillis, + }).pipe(Effect.orDie); + yield* store + .set(storeName(ref), new TextEncoder().encode(encoded)) + .pipe( + Effect.mapError((cause) => new SecretRequestError({ reason: "store_failed", cause })), + ); + }); + + /** Every field's value from a save, or none when a field is missing or extra. */ + const answerValues = ( + item: { readonly fields?: ReadonlyArray<{ readonly name: string }> | undefined }, + answer: Extract, + ): Record | undefined => { + const names = item.fields?.map((field) => field.name) ?? [SECRET_REQUEST_SINGLE_FIELD_NAME]; + const values = + "secret" in answer ? { [SECRET_REQUEST_SINGLE_FIELD_NAME]: answer.secret } : answer.secrets; + const keys = Object.keys(values); + return keys.length === names.length && names.every((name) => Object.hasOwn(values, name)) + ? values + : undefined; + }; + + const answerLock = yield* Semaphore.make(1); const answer: SecretRequests["Service"]["answer"] = (input) => Effect.gen(function* () { yield* Effect.annotateCurrentSpan({ @@ -96,27 +204,25 @@ const make = Effect.gen(function* () { if (run === undefined || ThreadManagementService.isTerminalRunStatus(run.status)) { return yield* new SecretRequestError({ reason: "agent_stopped" }); } - // Store first: the card only says saved once the value is kept. Create, - // not set: a second answer racing this one must not replace the value. - // A value already there on a card still pending is an earlier save whose - // record failed and could not be cleaned up; recording it finishes that save. + const ref = refFor(salt, input.threadId, item.id); + // Store first: the card only says saved once the value is kept. if (input.answer.type === "save") { - const encoded = yield* encodeStored({ - projectId: records.thread.projectId, - value: input.answer.secret, - savedAt: yield* Clock.currentTimeMillis, - }).pipe(Effect.orDie); - yield* store - .create( - storeName(refFor(salt, input.threadId, item.id)), - new TextEncoder().encode(encoded), - ) - .pipe( - Effect.catchIf(ServerSecretStore.isSecretAlreadyExistsError, () => Effect.void), - Effect.mapError( - (error) => new SecretRequestError({ reason: "store_failed", cause: error }), - ), - ); + const values = answerValues(item, input.answer); + if (values === undefined) { + return yield* new SecretRequestError({ reason: "fields_mismatch" }); + } + // A ref carries one value, so only a one-field request gets one. + const [single, ...rest] = Object.values(values); + if (single !== undefined && rest.length === 0) { + yield* storeSingle(ref, records.thread.projectId, single); + } + yield* writeFiles(ref, values).pipe( + Effect.mapError( + (error) => new SecretRequestError({ reason: "store_failed", cause: error }), + ), + // No ref without its file: the card stays pending for another save. + Effect.tapError(() => removeLogged(storeName(ref))), + ); } const secretStatus = input.answer.type === "save" ? "saved" : "declined"; yield* threadManagement @@ -130,6 +236,7 @@ const make = Effect.gen(function* () { label: item.label, reason: item.reason, ...(item.placeholder === undefined ? {} : { placeholder: item.placeholder }), + ...(item.fields === undefined ? {} : { fields: item.fields }), secretStatus, }) .pipe( @@ -138,7 +245,7 @@ const make = Effect.gen(function* () { // stored above would make that retry look already answered. Effect.tapError(() => input.answer.type === "save" - ? removeLogged(storeName(refFor(salt, input.threadId, item.id))) + ? Effect.andThen(removeLogged(storeName(ref)), removeFiles(ref)) : Effect.void, ), ); @@ -156,16 +263,35 @@ const make = Effect.gen(function* () { ); const card = recorded.turnItems.find((candidate) => candidate.id === item.id); if (card?.type === "secret_request" && card.secretStatus !== "saved") { - yield* removeLogged(storeName(refFor(salt, input.threadId, item.id))); + yield* removeLogged(storeName(ref)); + yield* removeFiles(ref); return yield* new SecretRequestError({ reason: "agent_stopped" }); } - }).pipe(Effect.withSpan("SecretRequests.answer")); + }).pipe( + // One answer at a time, across every request: two clients answering the + // same card see one answer win and the other already answered, and an + // answer's cleanup can never delete what another answer saved. Answers + // are rare human actions, so one lock costs nothing. + answerLock.withPermits(1), + Effect.withSpan("SecretRequests.answer"), + ); - const savedRef: SecretRequests["Service"]["savedRef"] = (input) => + const saved: SecretRequests["Service"]["saved"] = (input) => Effect.gen(function* () { const ref = refFor(salt, input.threadId, input.turnItemId); const stored = yield* store.get(storeName(ref)).pipe(Effect.orElseSucceed(Option.none)); - return Option.map(stored, () => ref); + const dir = filesDir(ref); + // Only field files: anything else an agent left in the directory is not a field. + const names = (yield* fileSystem + .readDirectory(dir) + .pipe(Effect.orElseSucceed((): Array => []))).filter((name) => + FIELD_FILE_PATTERN.test(name), + ); + if (Option.isNone(stored) && names.length === 0) return Option.none(); + return Option.some({ + files: Object.fromEntries(names.toSorted().map((name) => [name, path.join(dir, name)])), + ...(Option.isSome(stored) ? { secretRef: ref } : {}), + }); }); const removeLogged = (name: string) => @@ -237,14 +363,62 @@ const make = Effect.gen(function* () { Effect.catchCause((cause) => Effect.logWarning("Could not sweep expired secret refs", cause)), Effect.withSpan("SecretRequests.sweepExpired"), ); - yield* sweepExpired; - yield* sweepExpired.pipe( + /** + * Drops request files past the TTL, by their own write time, and request + * directories the agent already emptied. A save stopped midway leaves a + * temp directory that expires the same way. + */ + const sweepExpiredFiles = Effect.gen(function* () { + if (!(yield* fileSystem.exists(filesRoot))) return; + const now = yield* Clock.currentTimeMillis; + let removed = 0; + for (const entry of yield* fileSystem.readDirectory(filesRoot)) { + const match = FILES_DIR_PATTERN.exec(entry); + if (match === null) continue; + const dir = path.join(filesRoot, entry); + // One directory that cannot be read or removed must not keep the + // sweep from the rest; it is retried on the next sweep. + removed += yield* Effect.gen(function* () { + const expired = yield* Effect.gen(function* () { + const names = yield* fileSystem.readDirectory(dir); + if (names.length === 0 && match[1] === undefined) return true; + // An emptied temp directory has only its own time to go by. + const times = yield* Effect.forEach(names.length > 0 ? names : [""], (name) => + fileSystem + .stat(path.join(dir, name)) + .pipe( + Effect.map((info) => Option.getOrElse(Option.map(info.mtime, Number), () => 0)), + ), + ); + return now - Math.max(...times) > SECRET_REF_TTL_MS; + }); + if (!expired) return 0; + yield* fileSystem.remove(dir, { recursive: true, force: true }); + return 1; + }).pipe( + Effect.catch((error) => + Effect.logWarning("Could not sweep a secret request directory", { + errorTag: error._tag, + }).pipe(Effect.as(0)), + ), + ); + } + yield* Effect.annotateCurrentSpan({ "secret_request.expired_files_removed": removed }); + }).pipe( + Effect.catchCause((cause) => + Effect.logWarning("Could not sweep expired secret request files", cause), + ), + Effect.withSpan("SecretRequests.sweepExpiredFiles"), + ); + const sweep = Effect.andThen(sweepExpired, sweepExpiredFiles); + yield* sweep; + yield* sweep.pipe( Effect.delay("1 hour"), Effect.repeat(Schedule.spaced("1 hour")), Effect.forkScoped, ); - return SecretRequests.of({ answer, savedRef, consume }); + return SecretRequests.of({ answer, saved, consume }); }); export const layer = Layer.effect(SecretRequests, make); diff --git a/apps/web/src/components/chat/SecretRequestCard.tsx b/apps/web/src/components/chat/SecretRequestCard.tsx index 1c84c5f111..0b8d891a8f 100644 --- a/apps/web/src/components/chat/SecretRequestCard.tsx +++ b/apps/web/src/components/chat/SecretRequestCard.tsx @@ -2,8 +2,10 @@ import { SECRET_REQUEST_DEFAULT_PLACEHOLDER, SECRET_REQUEST_PRIVACY_NOTE, secretRequestAnswerInput, + secretRequestComplete, secretRequestDisplay, secretRequestFailureMessage, + secretRequestFields, type SecretRequestItem, } from "@t3tools/client-runtime/secret-request"; import { @@ -63,14 +65,19 @@ function PendingSecretRequestForm(props: { reportFailure: false, reportDefect: false, }); - const [secret, setSecret] = useState(""); + const fields = secretRequestFields(item); + const fieldId = (index: number) => `${inputId}-${index}`; + const [values, setValues] = useState>(() => new Map()); + const complete = secretRequestComplete(item, values); const [submitting, setSubmitting] = useState(false); const [error, setError] = useState(null); // Enter then a click can both run before a re-render; this guard is synchronous. const inFlight = useRef(false); const send = async ( - reply: { readonly type: "save"; readonly secret: string } | { readonly type: "decline" }, + reply: + | { readonly type: "save"; readonly values: ReadonlyMap } + | { readonly type: "decline" }, ) => { const input = secretRequestAnswerInput(item, reply); if (input === null || inFlight.current) return; @@ -82,7 +89,7 @@ function PendingSecretRequestForm(props: { setSubmitting(false); }); if (result._tag === "Success") { - setSecret(""); + setValues(new Map()); return; } if (!isAtomCommandInterrupted(result)) { @@ -92,7 +99,7 @@ function PendingSecretRequestForm(props: { const onSubmit = (event: FormEvent) => { event.preventDefault(); - void send({ type: "save", secret }); + void send({ type: "save", values }); }; return ( @@ -104,40 +111,58 @@ function PendingSecretRequestForm(props: { onPointerDown={(event) => { if (!(event.target instanceof HTMLElement)) return; if (event.target.closest("input, button, a")) return; - document.getElementById(inputId)?.focus(); + document.getElementById(fieldId(0))?.focus(); }} >
- + {/* With several fields, each input carries its own label. */} + {item.fields === undefined ? ( + + ) : ( +

{item.label}

+ )} {item.reason.trim() ?

{item.reason}

: null}
-
-
- setSecret(event.currentTarget.value)} - /> +
+
+ {fields.map((field, index) => ( +
+ {/* A single field is titled by the card itself. */} + {item.fields === undefined ? null : ( + + )} + { + const value = event.currentTarget.value; + setValues((current) => new Map(current).set(field.name, value)); + }} + /> +
+ ))}
-
diff --git a/docs/fork-features.md b/docs/fork-features.md index 4bc6cf7bdf..ce2c1a0d66 100644 --- a/docs/fork-features.md +++ b/docs/fork-features.md @@ -2729,3 +2729,40 @@ sources and reasons. "keywords": ["cookie mirror", "CookieMirror", "ChromeCookieSource", "preview:mirror-cookies"] } ``` + +## usable-secret-requests + +```json +{ + "id": "usable-secret-requests", + "purpose": "request_secret asks for 1-5 named fields on one card and returns a private 0600 file per field under the environment's state directory, plus a one-use secretRef for one-field requests. Answers run one at a time, values are kept exactly as typed, and files expire through the existing secret sweep. Drop when upstream returns usable secret values to agents.", + "issues": ["https://github.com/toolboxmd/chromeria/issues/550"], + "prs": ["https://github.com/toolboxmd/chromeria/pull/551"], + "newFiles": [], + "upstreamFiles": [ + "apps/mobile/src/features/threads/SecretRequestCard.tsx", + "apps/server/src/secrets/SecretRequests.test.ts", + "apps/server/src/secrets/SecretRequests.ts", + "apps/web/src/components/chat/SecretRequestCard.tsx", + "packages/client-runtime/src/secretRequest.test.ts", + "packages/client-runtime/src/secretRequest.ts", + "packages/contracts/src/orchestratorMcp.test.ts", + "packages/contracts/src/secretRequest.ts" + ], + "sharedFiles": [ + "apps/server/src/mcp/OrchestratorMcpService.ts", + "apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts", + "apps/server/src/mcp/toolkits/orchestrator/tools.ts", + "apps/server/src/orchestration-v2/Orchestrator.ts", + "packages/contracts/src/orchestrationV2.ts", + "packages/contracts/src/orchestratorMcp.ts", + "packages/provider-core/src/server/orchestrationInstructions.ts" + ], + "keywords": [ + "request_secret", + "secretRequestFields", + "SECRET_REQUEST_SINGLE_FIELD_NAME", + "secret-requests" + ] +} +``` diff --git a/packages/client-runtime/src/secretRequest.test.ts b/packages/client-runtime/src/secretRequest.test.ts index 3f8fe48885..9f89d389d3 100644 --- a/packages/client-runtime/src/secretRequest.test.ts +++ b/packages/client-runtime/src/secretRequest.test.ts @@ -47,19 +47,74 @@ describe("secretRequestDisplay", () => { }); describe("secretRequestAnswerInput", () => { - it("refuses a blank save and trims the value the server will store", () => { - expect(secretRequestAnswerInput(item, { type: "save", secret: " " })).toBeNull(); - expect(secretRequestAnswerInput(item, { type: "save", secret: " whsec_1 " })).toEqual({ + const single = { ...item, label: "Webhook secret" }; + it("refuses a blank save and sends the value exactly as typed", () => { + expect( + secretRequestAnswerInput(single, { type: "save", values: new Map([["secret", " "]]) }), + ).toBeNull(); + expect( + secretRequestAnswerInput(single, { + type: "save", + values: new Map([["secret", " whsec_1 "]]), + }), + ).toEqual({ threadId: item.threadId, turnItemId: item.id, - answer: { type: "save", secret: "whsec_1" }, + answer: { type: "save", secret: " whsec_1 " }, }); - expect(secretRequestAnswerInput(item, { type: "decline" })).toEqual({ + expect(secretRequestAnswerInput(single, { type: "decline" })).toEqual({ threadId: item.threadId, turnItemId: item.id, answer: { type: "decline" }, }); }); + + it("saves every field of a multi-field card together, or not at all", () => { + const card = { + ...single, + fields: [ + { name: "client_id", label: "Client ID" }, + { name: "client_secret", label: "Client secret" }, + ], + }; + expect( + secretRequestAnswerInput(card, { type: "save", values: new Map([["client_id", "id-1"]]) }), + ).toBeNull(); + expect( + secretRequestAnswerInput(card, { + type: "save", + values: new Map([ + ["client_id", " id-1 "], + ["client_secret", "cs-2"], + ["stray", "x"], + ]), + }), + ).toEqual({ + threadId: item.threadId, + turnItemId: item.id, + answer: { type: "save", secrets: { client_id: " id-1 ", client_secret: "cs-2" } }, + }); + }); + + it("reads fields named like Object properties from their own values", () => { + const card = { + ...single, + fields: [ + { name: "constructor", label: "Constructor" }, + { name: "tostring", label: "To string" }, + ], + }; + expect(secretRequestAnswerInput(card, { type: "save", values: new Map() })).toBeNull(); + expect( + secretRequestAnswerInput(card, { + type: "save", + values: new Map([ + ["constructor", "c-1"], + ["tostring", "t-2"], + ]), + })?.answer, + ).toEqual({ type: "save", secrets: { constructor: "c-1", tostring: "t-2" } }); + }); }); describe("secretRequestFailureMessage", () => { diff --git a/packages/client-runtime/src/secretRequest.ts b/packages/client-runtime/src/secretRequest.ts index 91640c0958..090d932e11 100644 --- a/packages/client-runtime/src/secretRequest.ts +++ b/packages/client-runtime/src/secretRequest.ts @@ -1,11 +1,16 @@ -import type { OrchestrationV2TurnItem, SecretRequestAnswerInput } from "@t3tools/contracts"; +import { + SECRET_REQUEST_SINGLE_FIELD_NAME, + type OrchestrationV2TurnItem, + type SecretRequestAnswerInput, + type SecretRequestField, +} from "@t3tools/contracts"; export type SecretRequestItem = Extract< OrchestrationV2TurnItem, { readonly type: "secret_request" } >; -export const SECRET_REQUEST_PRIVACY_NOTE = "Stored securely, never shown to the agent"; +export const SECRET_REQUEST_PRIVACY_NOTE = "Sent privately to the agent, never shown in chat"; export const SECRET_REQUEST_DEFAULT_PLACEHOLDER = "Paste the secret"; export type SecretRequestDisplay = @@ -58,16 +63,58 @@ export function secretRequestDisplay( } } +/** The inputs a card shows: its fields, or one field titled by the request itself. */ +export function secretRequestFields( + item: Pick, +): ReadonlyArray { + return ( + item.fields ?? [ + { + name: SECRET_REQUEST_SINGLE_FIELD_NAME, + label: item.label, + ...(item.placeholder === undefined ? {} : { placeholder: item.placeholder }), + }, + ] + ); +} + +/** Whether every field has a value; a blank one keeps Save disabled. */ +export function secretRequestComplete( + item: Pick, + values: ReadonlyMap, +): boolean { + return secretRequestFields(item).every( + (field) => (values.get(field.name) ?? "").trim().length > 0, + ); +} + +/** + * Builds the RPC payload for an answer from values keyed by field name (a Map, + * so a field named like an Object property reads its own value). Values are + * sent exactly as typed. A save with any blank field returns null, since the + * server saves all fields or none; callers keep Save disabled instead. + */ export function secretRequestAnswerInput( - item: Pick, - answer: { readonly type: "save"; readonly secret: string } | { readonly type: "decline" }, + item: Pick, + answer: + | { readonly type: "save"; readonly values: ReadonlyMap } + | { readonly type: "decline" }, ): SecretRequestAnswerInput | null { if (answer.type === "decline") { return { threadId: item.threadId, turnItemId: item.id, answer: { type: "decline" } }; } - const secret = answer.secret.trim(); - if (secret.length === 0) return null; - return { threadId: item.threadId, turnItemId: item.id, answer: { type: "save", secret } }; + if (!secretRequestComplete(item, answer.values)) return null; + const entries = secretRequestFields(item).map( + (field) => [field.name, answer.values.get(field.name) ?? ""] as const, + ); + const secrets = Object.fromEntries(entries); + const secret = item.fields === undefined ? entries[0]?.[1] : undefined; + // A card without fields is answered as servers before multi-field cards expect. + return { + threadId: item.threadId, + turnItemId: item.id, + answer: secret !== undefined ? { type: "save", secret } : { type: "save", secrets }, + }; } const USER_FACING_FAILURE_TAGS = new Set(["SecretRequestError", "EnvironmentAuthorizationError"]); diff --git a/packages/contracts/src/orchestrationV2.ts b/packages/contracts/src/orchestrationV2.ts index 733a53a11d..f32d1fdfc3 100644 --- a/packages/contracts/src/orchestrationV2.ts +++ b/packages/contracts/src/orchestrationV2.ts @@ -12,6 +12,7 @@ import { SavedPurposeIntent, } from "./executionPurpose.ts"; import { OrchestrationMessageContext } from "./composerContext.ts"; +import { SecretRequestField } from "./secretRequest.ts"; import * as Effect from "effect/Effect"; import * as Schema from "effect/Schema"; import * as SchemaAST from "effect/SchemaAST"; @@ -1574,6 +1575,8 @@ const OrchestrationV2SecretRequestFields = { label: TrimmedNonEmptyString, reason: Schema.String, placeholder: Schema.optional(Schema.String), + /** Absent for a single-field request, which asks for `label` alone. */ + fields: Schema.optional(Schema.Array(SecretRequestField)), secretStatus: OrchestrationV2SecretRequestStatus, } as const; @@ -3474,6 +3477,7 @@ const OrchestrationV2InternalCommand = Schema.Union([ label: TrimmedNonEmptyString, reason: Schema.String, placeholder: Schema.optional(Schema.String), + fields: Schema.optional(Schema.Array(SecretRequestField)), secretStatus: OrchestrationV2SecretRequestStatus, }), ]); diff --git a/packages/contracts/src/orchestratorMcp.test.ts b/packages/contracts/src/orchestratorMcp.test.ts index e6e46a2346..25303a35dd 100644 --- a/packages/contracts/src/orchestratorMcp.test.ts +++ b/packages/contracts/src/orchestratorMcp.test.ts @@ -5,6 +5,7 @@ import { OrchestratorMcpCreateThreadsInput, OrchestratorMcpDelegateTaskInput, OrchestratorMcpDelegateTaskResult, + OrchestratorMcpRequestSecretInput, OrchestratorMcpThreadInterruptInput, OrchestratorMcpThreadListInput, OrchestratorMcpThreadReadInput, @@ -29,6 +30,26 @@ describe("orchestrator MCP contracts", () => { ).toBe("worktree"); expect(() => decodeDelegateTaskInput({ task: "Review changes", isolation: "copy" })).toThrow(); }); + + it("accepts 1-5 uniquely named secret fields that can each be a file name", () => { + const decode = Schema.decodeUnknownOption(OrchestratorMcpRequestSecretInput); + const ask = (fields: ReadonlyArray<{ name: string; label: string }>) => + decode({ label: "Credentials", reason: "For the deploy job.", fields }); + const field = (name: string) => ({ name, label: name }); + expect(ask([field("client_id"), field("client-secret")])._tag).toBe("Some"); + expect(ask([field("constructor")])._tag).toBe("Some"); + for (const fields of [ + [], + [field("a"), field("a")], + [1, 2, 3, 4, 5, 6].map((index) => field(`f${index}`)), + [field("../x")], + [field("Token")], + [field("__proto__")], + ]) { + expect(ask(fields)._tag).toBe("None"); + } + }); + it("decodes cross-provider delegated task requests and durable results", () => { const request = decodeDelegateTaskInput({ task: "Inspect the workspace and report the result.", diff --git a/packages/contracts/src/orchestratorMcp.ts b/packages/contracts/src/orchestratorMcp.ts index 43a54a6e60..009ad7b3e6 100644 --- a/packages/contracts/src/orchestratorMcp.ts +++ b/packages/contracts/src/orchestratorMcp.ts @@ -37,6 +37,7 @@ import { import { ProviderInteractionMode, RuntimeMode } from "./providerPolicy.ts"; import * as ScheduledTaskFork from "./scheduledTaskChecks.ts"; import { ThreadLinkedPullRequest } from "./threadPullRequest.ts"; +import { SecretRequestField, SecretRequestFieldName } from "./secretRequest.ts"; import { ThreadTitleRegeneration } from "./threadTitle.ts"; import { OrchestrationV2ThreadLaunchWorkspaceStrategy, @@ -761,7 +762,21 @@ export const OrchestratorMcpRequestSecretInput = Schema.Struct({ "One or two sentences on what it is for and where the user gets or also enters it.", }), placeholder: Schema.optional(TrimmedNonEmptyString).annotate({ - description: "Hint inside the input, e.g. 'Paste your GitHub token'.", + description: + "Hint inside the input, e.g. 'Paste your GitHub token'. Single-field requests only; each entry in fields has its own.", + }), + fields: Schema.optional( + Schema.Array(SecretRequestField).check( + Schema.isMinLength(1), + Schema.isMaxLength(5), + Schema.makeFilter( + (fields) => new Set(fields.map((field) => field.name)).size === fields.length, + { expected: "fields with unique names" }, + ), + ), + ).annotate({ + description: + "Ask for several values on one card, e.g. [{name:'client_id', label:'Client ID'}, {name:'client_secret', label:'Client secret'}]. 1-5 entries; names are unique, lowercase [a-z0-9_-], and become the keys of files. Omit for one value.", }), timeoutMs: Schema.optional( Schema.Int.check(Schema.isBetween({ minimum: 1_000, maximum: 60 * 60 * 1_000 })), @@ -775,10 +790,16 @@ export type OrchestratorMcpRequestSecretInput = typeof OrchestratorMcpRequestSec export const OrchestratorMcpRequestSecretResult = Schema.Union([ Schema.Struct({ - status: Schema.Literal("saved").annotate({ description: "secretRef holds the value." }), - secretRef: SecretRef.annotate({ + status: Schema.Literal("saved").annotate({ + description: "files holds the values; secretRef too for a single-field request.", + }), + files: Schema.Record(SecretRequestFieldName, TrimmedNonEmptyString).annotate({ + description: + "Absolute path of a private file per field name ('secret' without fields), holding exactly that value. Use it without printing it, e.g. \"$(cat path)\" or stdin, and delete the file when done. It expires after 24 hours.", + }), + secretRef: Schema.optional(SecretRef).annotate({ description: - "Pass it to a tool that accepts a secretRef; it works once, and you never see the value.", + "Single-field requests only. Pass it to a tool that accepts a secretRef (schedule_task's signature.secretRef); it works once.", }), }), Schema.Struct({ diff --git a/packages/contracts/src/secretRequest.ts b/packages/contracts/src/secretRequest.ts index 3436fe0877..54709ac37f 100644 --- a/packages/contracts/src/secretRequest.ts +++ b/packages/contracts/src/secretRequest.ts @@ -2,15 +2,47 @@ import * as Schema from "effect/Schema"; import { ThreadId, TrimmedNonEmptyString, TurnItemId } from "./baseSchemas.ts"; +/** A field's name doubles as its file name, so it stays a plain lowercase word. */ +export const SecretRequestFieldName = TrimmedNonEmptyString.check( + Schema.isMaxLength(64), + Schema.isPattern(/^[a-z0-9_-]+$/), + // The one key a plain object cannot hold as its own property. + Schema.makeFilter((name: string) => name !== "__proto__", { expected: "not __proto__" }), +); + +/** A typed secret, kept exactly as entered: spaces can be part of a password. */ +const SecretValue = Schema.String.check( + Schema.makeFilter((value: string) => value.trim().length > 0, { + expected: "a non-blank secret", + }), +); + +/** One input on a secret request card. Carries what is asked, never the value. */ +export const SecretRequestField = Schema.Struct({ + name: SecretRequestFieldName, + label: TrimmedNonEmptyString, + placeholder: Schema.optional(TrimmedNonEmptyString), +}); +export type SecretRequestField = typeof SecretRequestField.Type; + +/** The name of the one field a request without `fields` asks for. */ +export const SECRET_REQUEST_SINGLE_FIELD_NAME = "secret"; + /** - * A saved value is kept by the server under a one-use SecretRef; the thread - * learns only the status. + * The user's answer to an agent's request for a secret. A card without + * `fields` is saved with `secret`; a card with `fields` is saved with every + * field's value in `secrets`, keyed by name. The server writes the values to + * private files for the agent; the thread learns only the status. */ export const SecretRequestAnswerInput = Schema.Struct({ threadId: ThreadId, turnItemId: TurnItemId, answer: Schema.Union([ - Schema.Struct({ type: Schema.Literal("save"), secret: TrimmedNonEmptyString }), + Schema.Struct({ type: Schema.Literal("save"), secret: SecretValue }), + Schema.Struct({ + type: Schema.Literal("save"), + secrets: Schema.Record(SecretRequestFieldName, SecretValue), + }), Schema.Struct({ type: Schema.Literal("decline") }), ]), }); @@ -21,6 +53,7 @@ const SECRET_REQUEST_FAILURE_MESSAGES = { not_found: "This secret request no longer exists.", already_answered: "This secret request was already answered.", agent_stopped: "The agent that asked has stopped, so this secret can't be used.", + fields_mismatch: "Fill in every field of this request.", store_failed: "Could not store the secret.", record_failed: "Saved the secret, but could not update the request.", invalid_ref: "That secretRef is not valid.", diff --git a/packages/provider-core/src/server/orchestrationInstructions.ts b/packages/provider-core/src/server/orchestrationInstructions.ts index 5b1b1a0c02..981c86407c 100644 --- a/packages/provider-core/src/server/orchestrationInstructions.ts +++ b/packages/provider-core/src/server/orchestrationInstructions.ts @@ -15,7 +15,7 @@ The \`t3-code\` MCP server provides app-owned orchestration. Treat these concept - \`t3_thread_send\` in its default \`auto\` mode does not interrupt a busy Claude thread: routine updates wait in its queue. Descendant-to-ancestor sends in the same environment use queued child notices and reject \`steer\` and \`restart\`. For an urgent course change to another permitted target, use \`mode: "restart"\` on a harness supporting restart. T3 requests the harness's native Stop cancellation, including background work within its scope, then delivers your message with the real requester and reason (a bare target gets your message alone). If cancellation fails after retries, the message is not delivered and the target's run shows why; Stop ends what can still be stopped. ACP and Antigravity background cancellation depends on the agent, and a tool shown interrupted may not have exited yet. Separately delegated T3 tasks keep running; cancel those explicitly. Restart needs a running turn; if the thread only waits on background work, use \`t3_thread_interrupt\`. Except on bare targets, restart rejects native slash commands; send the urgent instruction as text. Dispatch acceptance is not confirmation of cancellation or delivery, and interruption cannot undo finished effects. - \`schedule_task\` creates persistent recurring work in the app scheduler. Pass \`schedule\` as a structured object, never as JSON text: \`{"type":"interval","everyMs":3600000}\` for an interval, or \`{"type":"fixed_time","timeOfDay":"09:00","weekdays":[1,2,3,4,5]}\` for a wall-clock schedule, or \`{"type":"webhook"}\` to run on each request to the returned \`webhookUrl\` (the run sees the request only through \`{{body.path}}\`-style placeholders in the prompt). A run more than 55 minutes away (a later \`once\`, an \`interval\` longer than 55 minutes, \`fixed_time\`, or \`weekly\`) starts in a fresh thread by default, so write a self-contained prompt: Issue links, exact commands, what to report and where. Set \`bindToCurrentThread=true\` only when this thread will still be active, for example while you are coordinating live work; \`bindToCurrentThread=false\` forces a fresh thread. A \`once\` run within 55 minutes, an interval of 55 minutes or less, or a \`webhook\` still returns to this thread so you can delegate or dedupe. After scheduling a timer, report the returned cadence and next run time; for a webhook, report its \`webhookUrl\`, or say T3 Connect remote access is needed if it is missing. - \`t3_environment_links\` lists the user's other T3 Code environments that this one is linked to, such as another machine. To work there, pass that \`environmentId\` to \`t3_project_list\`, \`orchestrator_capabilities\`, \`t3_thread_launch\`, \`t3_thread_list\`, \`t3_thread_read\`, \`t3_thread_send\`, \`t3_thread_wait\`, or \`t3_thread_interrupt\`; project, thread, run, and model ids then belong to that environment. Work there runs within your own modes and the link's access. Set \`target.environmentId\` on \`delegate_task\` to delegate there; without \`target.model\`, the destination Prism chooses the model and prepares \`workspaceStrategy\` there. -- When you need a secret from the user (a token, API key, or webhook signing secret), call \`request_secret\` so they enter it privately, then pass the returned \`secretRef\` to the tool that needs it, e.g. \`signature.secretRef\` on a webhook task for a sender that signs requests such as GitHub. A \`secretRef\` works once. Never ask for a secret in chat, never invent one, and never repeat one. +- Whenever you need any secret from the user (a token, API key, password, login, or webhook signing secret), call \`request_secret\` so they enter it privately; pass \`fields\` to ask for several values, such as an ID and a key, on one card. The result's \`files\` maps each field to a private file holding its value: use it without printing it (\`"$(cat )"\`, or stdin such as \`bao kv put =-\` or \`rbw add\`), and delete the files when done. For a webhook task whose sender signs requests, such as GitHub, pass a single-field request's \`secretRef\` as \`signature.secretRef\`; it works once. Never ask for a secret in chat, never invent one, and never repeat one. - To mention another thread to the user, link it as \`[title](t3-thread://v1/)\` with its exact \`threadId\`, not URL-encoded. T3 Code opens the thread in the app and shows its current title. ### Choose the workspace before starting a new thread diff --git a/scripts/fork-upstream-edits.txt b/scripts/fork-upstream-edits.txt index 18f588f8cb..8f8b1ba7af 100644 --- a/scripts/fork-upstream-edits.txt +++ b/scripts/fork-upstream-edits.txt @@ -557,3 +557,11 @@ apps/web/src/components/settings/IntegrationsSettings.browser.test.tsx apps/web/src/hooks/useSettings.test.ts packages/shared/src/authScopeOptions.ts apps/web/vite.config.ts +apps/mobile/src/features/threads/SecretRequestCard.tsx +apps/server/src/secrets/SecretRequests.test.ts +apps/server/src/secrets/SecretRequests.ts +apps/web/src/components/chat/SecretRequestCard.tsx +packages/client-runtime/src/secretRequest.test.ts +packages/client-runtime/src/secretRequest.ts +packages/contracts/src/orchestratorMcp.test.ts +packages/contracts/src/secretRequest.ts