From ef95d8f55ef4a7191fe6af5686ecd80941fb0dca Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 26 Aug 2026 08:10:16 -0700 Subject: [PATCH] feat(secrets): agent-requested credentials the model never sees (#129670) * feat(secrets): agent-requested credentials the model never sees The new main-session secrets tool lets the agent request a credential by name: the human enters the value in a masked question card (Control UI, /ask/ deep link, iOS/macOS/Android), and the gateway diverts the answer straight into the shared secret store at question.resolve. The record, broadcast, waitAnswer, tool result, transcript, and model context only ever carry a synthetic stored marker. - protocol: additive secretStore binding, secretStoreExisting replacement metadata, and resolve-time secretStoreAllowedHosts (since 2026.8) - gateway: store-bound question validation, admin-gated minting (blocks questions-scope self-answer escalation past secrets.store.set), shared redaction-first store write service reused by secrets.store.set - tool: secrets request/list/delete; write-only by design, delete carries verified agent runtime identity; channel delivery is link-only so chat text is never captured as a secret - Control UI: masked composer card with requester identity, store banner, editable allowed hosts, replacement warning, retry-on-validation-error, a standalone /ask/ page, and a named startup-JS baseline bump - mobile: SecureField / password transformation for isSecret questions, no answer echo in terminal summaries; new native string registered in the locale-refresh inventory (generated artifacts stay workflow-owned) - regression: claimed harness secret input stays out of session transcripts Live-proven on an isolated dev gateway: real model turn, masked entry via Playwright, value present only in secret_store_entries, absent from every transcript, log, and the DOM. * chore(protocol): regenerate protocol models and tool display * fix(cli): read image string options through a typed helper PR #129463 added four commander option narrowings in image.ts without SAFETY coverage, leaving the assertion-safety ratchet red (21 > 17) for every branch on current main. Replace the casts with a typeof-checked read so the assertions are removed rather than annotated; each value is still validated by its normalizer. SAFETY comments cannot work in this file: the ratchet's raw scanner never rescans template tokens, so comments after the first substitution template are unreadable to it. * chore(protocol): refresh Swift models against current main * chore(i18n): re-baseline the native inventory on current main * docs(secrets): state the default-on tool policy and how to disable it * fix(secrets): tell the model what the store actually does The shipped tool description named the three actions and nothing else, and no parameter carried a description. The model could not tell that request blocks a human, that reason is shown to that human, what secret and env select, or - the silent-failure case - that a secret stored with no allowedHosts can never be substituted, so a successful request could produce a permanently unusable credential. Move the description to the presets module beside ask_user and document every parameter. * refactor(agents): share one blocking-question lifecycle between tools ask_user and secrets each carried their own registration, wait, and cancel logic, and they had diverged: ask_user recovers an answer that lands between its wait timeout and the cancel, while secrets discarded it and reported no_answer even though the Gateway had already stored the credential. One shared canceller and answer reader fixes that race for both, folds the two divergent gateway-call types into one, and drops two type assertions in favour of the canonical record guard (ask_user's assertion baseline shrinks 11 -> 8). Net +49 production lines: the shared module costs more than the duplication it removes, and buys the correctness fix plus a single owner for question lifecycle. * fix(ui): keep the allowed-hosts field readable as an input Main's composer restructure moved the free-text input styling into the option-row context, so the store-request hosts field - which sits outside a row - lost its border and read as static text. It is the one field the operator is meant to review and edit before releasing a credential, so give it its own border and focus ring. * fix(secrets): close two credential-boundary holes in agent requests Requests are now protected-secret only. list renders env values, so an agent could request kind=env, watch a human type it into a masked box under a no-visibility promise, then read it straight back; the tool text even claimed values are never returned. Environment values stay operator -set in Settings or the CLI, where they are agent-readable by design. Store-bound questions are also bound to the run that requested them. The resolve path authorized only the answering client, so a terminated or replaced agent run could still have a credential written on its behalf - the recorded runId was provenance, not closure-bound authority. Minting now requires a runId and resolution revalidates that exact live run immediately before the store write, with no await in between, failing closed as QUESTION_REQUESTER_INACTIVE. Both reported by ClawSweeper as P1 credential-boundary findings. --- apps/.i18n/native-source.json | 2 + .../openclaw/app/gateway/GatewayProtocol.kt | 16 + .../openclaw/app/ui/chat/ChatQuestionCard.kt | 24 +- .../OpenClawChatUI/ChatQuestionCard.swift | 46 +- .../OpenClawKit/Resources/tool-display.json | 1 + .../OpenClawProtocol/GatewayModels.swift | 60 ++- config/assertion-safety-baseline.txt | 2 +- docs/docs.json | 1 + docs/gateway/secrets.md | 2 + docs/tools/secrets.md | 78 +++ .../protocol-schema-fragment-approvals.ts | 2 + .../gateway-protocol/src/schema/questions.ts | 21 + src/agents/core-tool-factory-descriptors.ts | 1 + .../run/attempt.queue-message.test.ts | 48 ++ ...ed-agent-subscribe.handlers.tools.start.ts | 58 ++- ...ded-agent-subscribe.handlers.tools.test.ts | 48 ++ .../openclaw-tools.registration.test.ts | 26 +- src/agents/openclaw-tools.registration.ts | 26 +- src/agents/openclaw-tools.ts | 15 + src/agents/tool-catalog.test.ts | 2 + src/agents/tool-catalog.ts | 8 + src/agents/tool-description-presets.ts | 12 + src/agents/tool-display-config.ts | 5 + src/agents/tools/ask-user-tool.ts | 114 ++--- .../tools/gateway-question-lifecycle.ts | 107 +++++ .../tools/gateway.runtime-identity.test.ts | 1 + src/agents/tools/gateway.ts | 1 + src/agents/tools/secrets-tool.test.ts | 426 +++++++++++++++++ src/agents/tools/secrets-tool.ts | 352 ++++++++++++++ src/cli/capability-cli/image.ts | 87 ++-- src/gateway/server-aux-handlers.ts | 94 ++-- src/gateway/server-methods/question.test.ts | 448 +++++++++++++++++- src/gateway/server-methods/question.ts | 205 +++++++- src/gateway/server-methods/secrets.test.ts | 23 +- src/gateway/server-methods/secrets.ts | 121 +++-- test/telegram-question-gateway.test.ts | 6 +- ui/src/app-route-paths.ts | 33 +- ui/src/app/app-root.ts | 21 + ui/src/app/approval-deep-link.test.ts | 53 ++- ui/src/app/approval-deep-link.ts | 78 ++- ui/src/app/bootstrap.ts | 6 +- ui/src/app/lazy-custom-element.ts | 8 + .../app/question-prompt-secret-store.test.ts | 275 +++++++++++ ui/src/app/question-prompt-secret-store.ts | 148 ++++++ ui/src/app/question-prompt.ts | 57 ++- ui/src/e2e/question-flow.e2e.test.ts | 177 ++++++- ui/src/i18n/locales/en.ts | 4 + .../pages/chat/chat-thread.question.test.ts | 23 + .../components/chat-question-card.test.ts | 78 +++ .../chat/components/chat-question-card.ts | 91 +++- .../question/question-page-registration.ts | 5 + ui/src/pages/question/question-page.ts | 233 +++++++++ ui/src/styles/approval-boot.css | 6 +- ui/src/styles/chat/question-card.css | 39 ++ 54 files changed, 3512 insertions(+), 312 deletions(-) create mode 100644 docs/tools/secrets.md create mode 100644 src/agents/tools/gateway-question-lifecycle.ts create mode 100644 src/agents/tools/secrets-tool.test.ts create mode 100644 src/agents/tools/secrets-tool.ts create mode 100644 ui/src/app/question-prompt-secret-store.test.ts create mode 100644 ui/src/app/question-prompt-secret-store.ts create mode 100644 ui/src/pages/question/question-page-registration.ts create mode 100644 ui/src/pages/question/question-page.ts diff --git a/apps/.i18n/native-source.json b/apps/.i18n/native-source.json index 8e5cee99bfda..b56f733be3d1 100644 --- a/apps/.i18n/native-source.json +++ b/apps/.i18n/native-source.json @@ -1214,6 +1214,7 @@ {"id":"native.android.e23fb45cd6a06cd3","source":"Searching","surface":"android","sites":[{"kind":"ui-call","path":"apps/android/app/src/main/java/ai/openclaw/app/ui/SkillsSettingsScreen.kt"}]}, {"id":"native.android.1f09d9e9f3a237c1","source":"Searching threads","surface":"android","sites":[{"kind":"ui-call","path":"apps/android/app/src/main/java/ai/openclaw/app/ui/SessionsScreen.kt"},{"kind":"ui-call","path":"apps/android/app/src/main/java/ai/openclaw/app/ui/SidebarContent.kt"}]}, {"id":"native.android.e5387d6aaf34ede8","source":"Searching…","surface":"android","sites":[{"kind":"ui-state-text","path":"apps/android/app/src/main/java/ai/openclaw/app/MainViewModel.kt"},{"kind":"ui-state-text","path":"apps/android/app/src/main/java/ai/openclaw/app/gateway/GatewayDiscovery.kt"}]}, + {"id":"native.android.5616a95a172222e1","source":"Secret value","surface":"android","sites":[{"kind":"ui-call","path":"apps/android/app/src/main/java/ai/openclaw/app/ui/chat/ChatQuestionCard.kt"}]}, {"id":"native.android.2585b0fd6b35688c","source":"Secure (TLS)","surface":"android","sites":[{"kind":"ui-call","path":"apps/android/app/src/main/java/ai/openclaw/app/ui/OnboardingFlow.kt"},{"kind":"ui-call","path":"apps/android/app/src/main/java/ai/openclaw/app/ui/SettingsScreens.kt"}]}, {"id":"native.android.19749b2753923dd1","source":"Secure connection is required for this host.","surface":"android","sites":[{"kind":"ui-call","path":"apps/android/app/src/main/java/ai/openclaw/app/ui/GatewayConfigResolver.kt"}]}, {"id":"native.android.e72010e7aa8ccaee","source":"Security","surface":"android","sites":[{"kind":"resource-string","path":"apps/android/wear/src/main/res/values/strings.xml"}]}, @@ -3510,6 +3511,7 @@ {"id":"native.apple.c175b49ad1870aa3","source":"Searching ClawHub…","surface":"apple","sites":[{"kind":"ui-named-argument","path":"apps/macos/Sources/OpenClaw/ClawHubSkillsBrowser.swift"}]}, {"id":"native.apple.a9c4ded8f743bc25","source":"Searching…","surface":"apple","sites":[{"kind":"ui-localized-call","path":"apps/shared/OpenClawKit/Sources/OpenClawKit/GatewayDiscoveryStatusText.swift"}]}, {"id":"native.apple.526b0db228c6610c","source":"Seconds (optional)","surface":"apple","sites":[{"kind":"ui-call","path":"apps/macos/Sources/OpenClaw/CronJobEditor.swift"}]}, + {"id":"native.apple.eb87a5d402f5de88","source":"Secret value","surface":"apple","sites":[{"kind":"ui-call","path":"apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatQuestionCard.swift"},{"kind":"ui-modifier","path":"apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatQuestionCard.swift"}]}, {"id":"native.apple.1c5b74f182a37abe","source":"Secure","surface":"apple","sites":[{"kind":"conditional-branch","path":"apps/ios/Sources/Gateway/GatewayQuickSetupSheet.swift"}]}, {"id":"native.apple.03491d2d66766168","source":"Secure (TLS)","surface":"apple","sites":[{"kind":"ui-call","path":"apps/ios/Sources/Design/SettingsProTabSections.swift"},{"kind":"ui-call","path":"apps/ios/Sources/Onboarding/OnboardingWizardView.swift"}]}, {"id":"native.apple.25f10dfdc6cd5627","source":"Secure connection is required for this host.","surface":"apple","sites":[{"kind":"ui-localized-call","path":"apps/ios/Sources/Gateway/GatewayConnectionController+Capabilities.swift"}]}, diff --git a/apps/android/app/src/main/java/ai/openclaw/app/gateway/GatewayProtocol.kt b/apps/android/app/src/main/java/ai/openclaw/app/gateway/GatewayProtocol.kt index f91dd5b17344..0587ae431e0e 100644 --- a/apps/android/app/src/main/java/ai/openclaw/app/gateway/GatewayProtocol.kt +++ b/apps/android/app/src/main/java/ai/openclaw/app/gateway/GatewayProtocol.kt @@ -91,6 +91,8 @@ data class Question( val multiSelect: Boolean? = null, val isOther: Boolean? = null, val isSecret: Boolean? = null, + val secretStore: QuestionSecretStore? = null, + val secretStoreExisting: QuestionSecretStoreExisting? = null, ) @Serializable @@ -344,6 +346,20 @@ data class GatewayNodeInvokeResultParamsError( val message: String? = null, ) +@Serializable +data class QuestionSecretStore( + val name: String, + val kind: String, + val allowedHosts: List? = null, + val reason: String? = null, +) + +@Serializable +data class QuestionSecretStoreExisting( + val updatedAtMs: Long, + val updatedBy: String? = null, +) + @Serializable data class ProjectsListResultProjectsItem( val id: String, diff --git a/apps/android/app/src/main/java/ai/openclaw/app/ui/chat/ChatQuestionCard.kt b/apps/android/app/src/main/java/ai/openclaw/app/ui/chat/ChatQuestionCard.kt index 213f4bfe9862..dcc82c337b59 100644 --- a/apps/android/app/src/main/java/ai/openclaw/app/ui/chat/ChatQuestionCard.kt +++ b/apps/android/app/src/main/java/ai/openclaw/app/ui/chat/ChatQuestionCard.kt @@ -15,6 +15,7 @@ import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.width import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.foundation.text.KeyboardOptions import androidx.compose.material3.Button import androidx.compose.material3.Checkbox import androidx.compose.material3.OutlinedTextField @@ -32,6 +33,9 @@ import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.text.font.FontWeight +import androidx.compose.ui.text.input.KeyboardType +import androidx.compose.ui.text.input.PasswordVisualTransformation +import androidx.compose.ui.text.input.VisualTransformation import androidx.compose.ui.unit.dp import kotlinx.coroutines.delay @@ -165,14 +169,21 @@ private fun QuestionSection( } } if (question.options.isEmpty() || question.isOther == true) { + // Secret answers must never render on screen or feed the keyboard's + // prediction/autocorrect stores; password transformation + keyboard type + // cover both, and single-line keeps the masked value one obscured run. + val secret = question.isSecret == true OutlinedTextField( value = draft.otherText[question.questionId].orEmpty(), onValueChange = { onDraftChanged(draft.setOther(question, it)) }, modifier = Modifier.fillMaxWidth(), enabled = enabled, - label = { Text(nativeString("Other answer")) }, + label = { Text(if (secret) nativeString("Secret value") else nativeString("Other answer")) }, + visualTransformation = if (secret) PasswordVisualTransformation() else VisualTransformation.None, + keyboardOptions = + if (secret) KeyboardOptions(keyboardType = KeyboardType.Password) else KeyboardOptions.Default, minLines = 1, - maxLines = 4, + maxLines = if (secret) 1 else 4, ) } } @@ -229,8 +240,13 @@ internal fun terminalQuestionAnswer( if (status == ChatQuestionStatus.Cancelled) return nativeString("Skipped") if (status == ChatQuestionStatus.Expired) return nativeString("Expired") if (status == ChatQuestionStatus.Unavailable) return nativeString("Unavailable") - prompt.record.answers?.answers?.get(question.questionId)?.takeIf { it.isNotEmpty() }?.let { - return it.joinToString(", ") + // Secret questions never echo answer text into the persisted timeline; the + // record only carries a synthetic marker, but masking here keeps the summary + // honest for every secret producer, not just store-bound ones. + if (question.isSecret != true) { + prompt.record.answers?.answers?.get(question.questionId)?.takeIf { it.isNotEmpty() }?.let { + return it.joinToString(", ") + } } return if (status == ChatQuestionStatus.AnsweredElsewhere) nativeString("Answered elsewhere") else nativeString("Answered") } diff --git a/apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatQuestionCard.swift b/apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatQuestionCard.swift index b2eeff7f4259..e0d1eb28d797 100644 --- a/apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatQuestionCard.swift +++ b/apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatQuestionCard.swift @@ -256,11 +256,17 @@ public final class OpenClawQuestionCardModel: Identifiable { } public func terminalSummaryText(for question: Question) -> String { - switch self.status() { + // Secret questions never echo answer text into the persisted timeline; + // the record only carries a synthetic marker, but masking here keeps the + // summary honest for every secret producer, not just store-bound ones. + let echoedAnswers = question.issecret == true + ? nil + : self.answerValues(questionID: question.questionid)?.joined(separator: ", ") + return switch self.status() { case .answered: - self.answerValues(questionID: question.questionid)?.joined(separator: ", ") ?? String(localized: "Answered") + echoedAnswers ?? String(localized: "Answered") case .answeredElsewhere: - self.answerValues(questionID: question.questionid)?.joined(separator: ", ") + echoedAnswers ?? String(localized: "Answered elsewhere") case .cancelled: String(localized: "Skipped") @@ -377,16 +383,30 @@ struct OpenClawQuestionCard: View { self.optionRow(question: question, option: option, now: now) } if question.options.isEmpty || question.isother == true { - TextField( - "Other answer", - text: Binding( - get: { self.model.otherText[question.questionid] ?? "" }, - set: { self.model.setOtherText(questionID: question.questionid, value: $0) }), - axis: .vertical) - .font(OpenClawChatTypography.body) - .textFieldStyle(.roundedBorder) - .disabled(self.model.status(at: now) != .pending) - .accessibilityLabel("Other answer") + if question.issecret == true { + // Secret answers must never render on screen: masked entry, no + // autocorrect/prediction capture, same submit path as free text. + SecureField( + "Secret value", + text: Binding( + get: { self.model.otherText[question.questionid] ?? "" }, + set: { self.model.setOtherText(questionID: question.questionid, value: $0) })) + .font(OpenClawChatTypography.body) + .textFieldStyle(.roundedBorder) + .disabled(self.model.status(at: now) != .pending) + .accessibilityLabel("Secret value") + } else { + TextField( + "Other answer", + text: Binding( + get: { self.model.otherText[question.questionid] ?? "" }, + set: { self.model.setOtherText(questionID: question.questionid, value: $0) }), + axis: .vertical) + .font(OpenClawChatTypography.body) + .textFieldStyle(.roundedBorder) + .disabled(self.model.status(at: now) != .pending) + .accessibilityLabel("Other answer") + } } } #if os(macOS) diff --git a/apps/shared/OpenClawKit/Sources/OpenClawKit/Resources/tool-display.json b/apps/shared/OpenClawKit/Sources/OpenClawKit/Resources/tool-display.json index 1623638546f2..5ae04f461c73 100644 --- a/apps/shared/OpenClawKit/Sources/OpenClawKit/Resources/tool-display.json +++ b/apps/shared/OpenClawKit/Sources/OpenClawKit/Resources/tool-display.json @@ -24,6 +24,7 @@ "update_goal": {"emoji":"🎯","title":"Update Goal","detailKeys":["status"]}, "progress_card": {"emoji":"🗺️","title":"Progress Card","detailKeys":["plan.0.step","markdown"]}, "ask_user": {"emoji":"❓","title":"Ask User","detailKeys":["questions.0.question"]}, + "secrets": {"emoji":"🔑","title":"Secrets","detailKeys":["action","name","kind"]}, "suggest_task": {"emoji":"✨","title":"Suggest Task","detailKeys":["title","tldr","cwd"]}, "dismiss_task": {"emoji":"🗑️","title":"Dismiss Task","detailKeys":["task_id","reason"]}, "skill_workshop": {"emoji":"🧰","title":"Skill Workshop","detailKeys":["action","name","proposal_id"]}, diff --git a/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift b/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift index c2476c623328..5533a3ba83a5 100644 --- a/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift +++ b/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift @@ -18362,6 +18362,50 @@ public struct QuestionOption: Codable, Sendable { } } +public struct QuestionSecretStoreBinding: Codable, Sendable { + public let name: String + public let kind: AnyCodable + public let allowedhosts: [String]? + public let reason: String? + + public init( + name: String, + kind: AnyCodable, + allowedhosts: [String]? = nil, + reason: String? = nil) + { + self.name = name + self.kind = kind + self.allowedhosts = allowedhosts + self.reason = reason + } + + private enum CodingKeys: String, CodingKey { + case name + case kind + case allowedhosts = "allowedHosts" + case reason + } +} + +public struct QuestionSecretStoreExisting: Codable, Sendable { + public let updatedatms: Int + public let updatedby: String? + + public init( + updatedatms: Int, + updatedby: String? = nil) + { + self.updatedatms = updatedatms + self.updatedby = updatedby + } + + private enum CodingKeys: String, CodingKey { + case updatedatms = "updatedAtMs" + case updatedby = "updatedBy" + } +} + public struct Question: Codable, Sendable { public let questionid: String public let header: String @@ -18370,6 +18414,8 @@ public struct Question: Codable, Sendable { public let multiselect: Bool? public let isother: Bool? public let issecret: Bool? + public let secretstore: QuestionSecretStoreBinding? + public let secretstoreexisting: QuestionSecretStoreExisting? public init( questionid: String, @@ -18378,7 +18424,9 @@ public struct Question: Codable, Sendable { options: [QuestionOption], multiselect: Bool? = nil, isother: Bool? = nil, - issecret: Bool? = nil) + issecret: Bool? = nil, + secretstore: QuestionSecretStoreBinding? = nil, + secretstoreexisting: QuestionSecretStoreExisting? = nil) { self.questionid = questionid self.header = header @@ -18387,6 +18435,8 @@ public struct Question: Codable, Sendable { self.multiselect = multiselect self.isother = isother self.issecret = issecret + self.secretstore = secretstore + self.secretstoreexisting = secretstoreexisting } private enum CodingKeys: String, CodingKey { @@ -18397,6 +18447,8 @@ public struct Question: Codable, Sendable { case multiselect = "multiSelect" case isother = "isOther" case issecret = "isSecret" + case secretstore = "secretStore" + case secretstoreexisting = "secretStoreExisting" } } @@ -18408,6 +18460,7 @@ public struct QuestionRequestQuestion: Codable, Sendable { public let multiselect: Bool? public let isother: Bool? public let issecret: Bool? + public let secretstore: QuestionSecretStoreBinding? public init( questionid: String, @@ -18416,7 +18469,8 @@ public struct QuestionRequestQuestion: Codable, Sendable { options: [QuestionOption], multiselect: Bool? = nil, isother: Bool? = nil, - issecret: Bool? = nil) + issecret: Bool? = nil, + secretstore: QuestionSecretStoreBinding? = nil) { self.questionid = questionid self.header = header @@ -18425,6 +18479,7 @@ public struct QuestionRequestQuestion: Codable, Sendable { self.multiselect = multiselect self.isother = isother self.issecret = issecret + self.secretstore = secretstore } private enum CodingKeys: String, CodingKey { @@ -18435,6 +18490,7 @@ public struct QuestionRequestQuestion: Codable, Sendable { case multiselect = "multiSelect" case isother = "isOther" case issecret = "isSecret" + case secretstore = "secretStore" } } diff --git a/config/assertion-safety-baseline.txt b/config/assertion-safety-baseline.txt index d8171d25179a..303e2034249e 100644 --- a/config/assertion-safety-baseline.txt +++ b/config/assertion-safety-baseline.txt @@ -2087,7 +2087,7 @@ src/agents/tools-effective-inventory.ts 4 src/agents/tools/agent-step.ts 6 src/agents/tools/agents-wait-tool.ts 1 src/agents/tools/ask-user-tool-normalization.ts 3 -src/agents/tools/ask-user-tool.ts 11 +src/agents/tools/ask-user-tool.ts 8 src/agents/tools/automations-tool-name.ts 1 src/agents/tools/chat-history-text.ts 3 src/agents/tools/common.ts 2 diff --git a/docs/docs.json b/docs/docs.json index da6ee1b8d4fe..dbad5645b893 100644 --- a/docs/docs.json +++ b/docs/docs.json @@ -1409,6 +1409,7 @@ "tools/music-generation", "tools/pdf", "tools/reactions", + "tools/secrets", "tools/thinking", "tools/tokenjuice", "tools/tool-search", diff --git a/docs/gateway/secrets.md b/docs/gateway/secrets.md index 8021f080a409..a37a6c4cc79e 100644 --- a/docs/gateway/secrets.md +++ b/docs/gateway/secrets.md @@ -303,6 +303,8 @@ Reference an entry from `openclaw.json` with the `store` source: Control UI set/delete operations automatically refresh the active secrets runtime when the changed name is referenced by a `store` SecretRef in the active source config. Names that are not referenced skip that work. Direct CLI writes remain an offline/local path; after changing a config-referenced value with the CLI, run `openclaw secrets reload` so the active in-memory snapshot picks it up. +The agent can also ask you to add an entry with the [`secrets` tool](/tools/secrets): it names the entry and the reason, you type the value into a masked prompt, and the Gateway writes it directly into the store. The value never enters the chat, the transcript, or the model's context, and the same automatic runtime refresh applies. + Store values are not encrypted at rest. They are stored unencrypted in the shared state SQLite database (`state/openclaw.sqlite`), protected by the same `0600` file and `0700` directory permissions as other credentials in that database. Operators who need stronger storage isolation should use an external exec provider such as the [1Password plugin](/plugins/onepassword) or [Vault SecretRefs](/plugins/vault). diff --git a/docs/tools/secrets.md b/docs/tools/secrets.md new file mode 100644 index 000000000000..db58da4d0482 --- /dev/null +++ b/docs/tools/secrets.md @@ -0,0 +1,78 @@ +--- +summary: "How the secrets tool lets the agent request credentials it never sees" +read_when: + - You want the agent to obtain an API key without it entering the chat + - You are answering or debugging a credential request prompt + - You need the secrets tool schema, storage, or channel behavior +title: "Secrets" +--- + +`secrets` lets the agent ask you for a credential without ever seeing it. The +agent names an entry, you type the value into a trusted prompt, and the Gateway +writes it straight into the shared secret store. The value never appears in the +chat, the session transcript, the tool result, or the model's context — the +agent only learns that the entry now exists. + +The tool is available only in the main session. Subagents and other +non-primary runs do not receive it. + +It is enabled by default and governed by the normal tool policy — there is no +dedicated config key. To remove it, deny it like any other tool (for example +`tools.deny: ["secrets"]` in `openclaw.json`); allowlists and tool profiles +apply to it the same way. Creating a credential request also requires an +`operator.admin` Gateway client, which the agent's own client satisfies. + +## Actions + +- `request` — ask the human for a credential and store it under a name such as + `STRIPE_API_KEY`. Requests are protected-secret only: an `env` value is + readable through `list`, so requesting one would break the promise the masked + prompt makes. The agent may propose `allowedHosts` and a short `reason` shown + on the prompt, and the tool blocks until you answer, skip, or it times out + (15 minutes by default). The request is bound to the requesting agent run; if + that run ends before you answer, the write is refused. +- `list` — entry metadata: name, kind, allowed hosts, and last update. Secret + values are structurally absent from the listing. Operator-set `env` entries + show their value, since those are injected into exec environments anyway and + are agent-readable by design. +- `delete` — soft-delete an entry by name. Deleted entries are purged after 30 + days. + +There is deliberately no action that writes a value the agent supplies. If a +value must enter the store, it arrives through the human prompt, the +`/settings/secrets` page, or the [`openclaw secrets store` CLI](/cli/secrets). + +## Answering a request + +The web Control UI docks the prompt above the composer with a masked input. +The prompt always shows who is asking (agent and session), the entry name and +kind, the agent's stated reason, and — for secret entries — an editable list of +allowed hosts, so you have the final say on where the credential may be used. +If the name already exists, the prompt says so and shows when and by whom the +entry was last updated; submitting replaces the stored value. + +iOS, macOS, and Android render the same card with a masked secret field. + +Chat channels never accept the value. On Telegram, Discord, and similar +surfaces the request is delivered as a link to the Control UI prompt — typing +a credential into a chat message is exactly what this flow exists to avoid, so +a plain-text reply is not captured as an answer. + +Creating a credential request requires an `operator.admin` client (the agent's +own Gateway client qualifies). Answering needs only the normal question scope, +because answering provides a value rather than reading one. + +## Using a stored credential + +A stored entry is a regular shared-store entry (see +[Secrets management](/gateway/secrets)): + +- Reference it from config as `{ "source": "store", "id": "STRIPE_API_KEY" }` + wherever a SecretRef is accepted (provider API keys, channel tokens). Writes + refresh affected config references automatically. +- `env` entries are injected into gateway-host exec environments starting with + the next agent run. +- `secret` entries are substituted into subprocess traffic only when the + egress proxy is enabled (`secrets.egressProxy.enabled`) and the destination + matches the entry's allowed hosts; the agent and its subprocesses otherwise + see only an opaque placeholder. diff --git a/packages/gateway-protocol/src/schema/protocol-schema-fragment-approvals.ts b/packages/gateway-protocol/src/schema/protocol-schema-fragment-approvals.ts index 069bd9740442..4347e97b2ec7 100644 --- a/packages/gateway-protocol/src/schema/protocol-schema-fragment-approvals.ts +++ b/packages/gateway-protocol/src/schema/protocol-schema-fragment-approvals.ts @@ -43,6 +43,8 @@ export const ApprovalProtocolSchemas = { ExecApprovalRequestParams: execApprovals.ExecApprovalRequestParamsSchema, ExecApprovalResolveParams: execApprovals.ExecApprovalResolveParamsSchema, QuestionOption: questions.QuestionOptionSchema, + QuestionSecretStoreBinding: questions.QuestionSecretStoreBindingSchema, + QuestionSecretStoreExisting: questions.QuestionSecretStoreExistingSchema, Question: questions.QuestionSchema, QuestionRequestQuestion: questions.QuestionRequestQuestionSchema, QuestionAnswers: questions.QuestionAnswersSchema, diff --git a/packages/gateway-protocol/src/schema/questions.ts b/packages/gateway-protocol/src/schema/questions.ts index bc011e93e13c..1c3b00c050de 100644 --- a/packages/gateway-protocol/src/schema/questions.ts +++ b/packages/gateway-protocol/src/schema/questions.ts @@ -8,12 +8,28 @@ import { withSince } from "./since.js"; const QuestionIdSchema = Type.String({ pattern: "^[a-z][a-z0-9_]*$" }); // UI chip/tag display cap shared by every question input and output shape. const QuestionHeaderSchema = Type.String({ maxLength: 12 }); +const QuestionSecretStoreAllowedHostsSchema = Type.Array( + Type.String({ minLength: 1, maxLength: 253 }), + { maxItems: 128, uniqueItems: true }, +); export const QuestionOptionSchema = closedObject({ label: NonEmptyString, description: Type.Optional(Type.String()), }); +export const QuestionSecretStoreBindingSchema = closedObject({ + name: Type.String({ minLength: 1, maxLength: 128, pattern: "^[A-Z][A-Z0-9_]{0,127}$" }), + kind: Type.Union([Type.Literal("secret"), Type.Literal("env")]), + allowedHosts: Type.Optional(QuestionSecretStoreAllowedHostsSchema), + reason: Type.Optional(Type.String({ maxLength: 200 })), +}); + +export const QuestionSecretStoreExistingSchema = closedObject({ + updatedAtMs: Type.Integer({ minimum: 0 }), + updatedBy: Type.Optional(NonEmptyString), +}); + const QuestionInputFields = { questionId: QuestionIdSchema, header: QuestionHeaderSchema, @@ -22,6 +38,7 @@ const QuestionInputFields = { multiSelect: Type.Optional(Type.Boolean()), isOther: Type.Optional(Type.Boolean()), isSecret: Type.Optional(Type.Boolean()), + secretStore: Type.Optional(withSince("2026.8", QuestionSecretStoreBindingSchema)), }; /** Unnormalized question accepted by question.request. */ @@ -29,6 +46,7 @@ export const QuestionRequestQuestionSchema = closedObject(QuestionInputFields); const QuestionFields = { ...QuestionInputFields, + secretStoreExisting: Type.Optional(withSince("2026.8", QuestionSecretStoreExistingSchema)), }; /** Canonical normalized question shown to an operator. */ @@ -94,6 +112,9 @@ export const QuestionResolveParamsSchema = Type.Union([ closedObject({ id: NonEmptyString, answers: QuestionAnswersSchema, + secretStoreAllowedHosts: Type.Optional( + withSince("2026.8", QuestionSecretStoreAllowedHostsSchema), + ), resolvedBy: Type.Optional(NonEmptyString), }), closedObject({ diff --git a/src/agents/core-tool-factory-descriptors.ts b/src/agents/core-tool-factory-descriptors.ts index 43f3eeeb3a30..362693c05964 100644 --- a/src/agents/core-tool-factory-descriptors.ts +++ b/src/agents/core-tool-factory-descriptors.ts @@ -29,6 +29,7 @@ const CORE_TOOL_FACTORY_DESCRIPTORS = [ { name: "conversations_turn", family: "openclaw" }, { name: AUTOMATIONS_TOOL_NAME, family: "openclaw" }, { name: "screen", family: "openclaw" }, + { name: "secrets", family: "openclaw" }, { name: "dashboard", family: "openclaw" }, { name: "gateway", family: "openclaw" }, { name: "get_goal", family: "openclaw" }, diff --git a/src/agents/embedded-agent-runner/run/attempt.queue-message.test.ts b/src/agents/embedded-agent-runner/run/attempt.queue-message.test.ts index 58513496d532..3cc09c2727ca 100644 --- a/src/agents/embedded-agent-runner/run/attempt.queue-message.test.ts +++ b/src/agents/embedded-agent-runner/run/attempt.queue-message.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from "vitest"; import { createUserTurnTranscriptRecorder } from "../../../sessions/user-turn-transcript.js"; import { createTestUserTurnTranscriptTarget } from "../../../sessions/user-turn-transcript.test-support.js"; +import { runAgentHarnessGatewayQuestion } from "../../harness/gateway-question.js"; import { registerQueuedUserMessageRetirement } from "../../sessions/queued-user-message-retirement.js"; import { reportSteeringMessagePersistenceFailure, @@ -45,6 +46,53 @@ function steerWithDeliveryWait( } describe("embedded OpenClaw queued steering cancellation", () => { + it("keeps a claimed harness secret out of the session transcript", async () => { + const secretValue = "test-secret-value-123"; + const sessionKey = "agent:main:secret-transcript"; + const persistedTranscript: string[] = []; + const recorder = createUserTurnTranscriptRecorder({ + input: { text: secretValue }, + target: createTestUserTurnTranscriptTarget({ sessionKey }), + }); + const persistApproved = vi.spyOn(recorder, "persistApproved").mockImplementation(async () => { + persistedTranscript.push(JSON.stringify(recorder.message?.content)); + return undefined; + }); + const onBlockReply = vi.fn(async () => undefined); + const pendingSecret = runAgentHarnessGatewayQuestion({ + questions: [ + { + id: "credential", + header: "API key", + question: "Enter the requested credential", + isSecret: true, + options: [], + }, + ], + sessionKey, + timeoutMs: 60_000, + gatewayCall: vi.fn(), + delivery: { onBlockReply }, + }); + const steer = vi.fn(async () => undefined); + + await steerActiveSessionWithOptionalDeliveryWait( + { steer, subscribe: () => () => {} }, + secretValue, + { isInboundUserMessage: true, userTurnTranscriptRecorder: recorder }, + sessionKey, + ); + + await expect(pendingSecret).resolves.toEqual({ + status: "answered", + answers: { answers: { credential: [secretValue] } }, + }); + expect(persistApproved).not.toHaveBeenCalled(); + expect(recorder.hasPersisted()).toBe(false); + expect(persistedTranscript.join("\n")).not.toContain(secretValue); + expect(steer).not.toHaveBeenCalled(); + }); + it("forwards prepared transcript context with a queued steering message", async () => { const steer = vi.fn(async () => undefined); const recorder = createUserTurnTranscriptRecorder({ diff --git a/src/agents/embedded-agent-subscribe.handlers.tools.start.ts b/src/agents/embedded-agent-subscribe.handlers.tools.start.ts index cfb10989644a..0b9b42b1d2ae 100644 --- a/src/agents/embedded-agent-subscribe.handlers.tools.start.ts +++ b/src/agents/embedded-agent-subscribe.handlers.tools.start.ts @@ -4,6 +4,9 @@ import { readStringValue, } from "@openclaw/normalization-core/string-coerce"; import { truncateUtf16Safe } from "@openclaw/normalization-core/utf16-slice"; +import { normalizeControlUiBasePath } from "@openclaw/session-url-contract"; +import { resolveControlUiSessionLinkBase } from "../config/control-ui-link-base.js"; +import { resolveGatewayPort } from "../config/paths.js"; import { emitAgentActivityEvent, type AgentItemEventData } from "../infra/agent-activity-events.js"; import { emitAgentEvent } from "../infra/agent-events.js"; import { REQUIRED_PARAM_GROUPS, type RequiredParamGroup } from "./agent-tools.params.js"; @@ -38,6 +41,7 @@ import { settleAskUserPromptDelivery, waitForAskUserPromptReady, } from "./tools/ask-user-tool.js"; +import { normalizeSecretsRequestParams } from "./tools/secrets-tool.js"; const TRACE_REQUIRED_PARAM_GROUPS = { read: [{ keys: ["path", "file_path"], label: "path" }], @@ -45,7 +49,8 @@ const TRACE_REQUIRED_PARAM_GROUPS = { edit: REQUIRED_PARAM_GROUPS.edit, } satisfies Record; -function buildAskUserPromptPayload( +function reserveQuestionPromptDelivery( + toolName: "ask_user" | "secrets", toolCallId: string, sessionKey: string | undefined, runId: string, @@ -53,7 +58,8 @@ function buildAskUserPromptPayload( args: unknown, ) { try { - const { questions, timeoutSeconds } = normalizeAskUserParams(args); + const { questions, timeoutSeconds } = + toolName === "secrets" ? normalizeSecretsRequestParams(args) : normalizeAskUserParams(args); const reservation = reserveAskUserPromptDelivery({ toolCallId, sessionKey, @@ -324,9 +330,17 @@ export function handleToolExecutionStart( ): void | Promise { const startToolName = normalizeToolPolicyName(evt.toolName); ctx.state.liveEditDiffStateById.delete(evt.toolCallId); - const askUserPromptReservation = - startToolName === "ask_user" && ctx.params.onToolResult - ? buildAskUserPromptPayload( + const isQuestionTool = + startToolName === "ask_user" || + (startToolName === "secrets" && + evt.args !== null && + typeof evt.args === "object" && + "action" in evt.args && + evt.args.action === "request"); + const questionPromptReservation = + isQuestionTool && ctx.params.onToolResult + ? reserveQuestionPromptDelivery( + startToolName === "ask_user" ? "ask_user" : "secrets", evt.toolCallId, ctx.params.sessionKey, ctx.params.runId, @@ -334,8 +348,8 @@ export function handleToolExecutionStart( evt.args, ) : undefined; - const cancelAskUserPromptReservation = () => { - if (askUserPromptReservation) { + const cancelQuestionPromptReservation = () => { + if (questionPromptReservation) { cancelAskUserPromptDelivery( evt.toolCallId, ctx.params.sessionKey, @@ -352,14 +366,14 @@ export function handleToolExecutionStart( assistantMessageIndex: ctx.state.assistantMessageIndex, }); } catch (error) { - cancelAskUserPromptReservation(); + cancelQuestionPromptReservation(); throw error; } if (isPromiseLike(onBlockReplyFlushResult)) { return onBlockReplyFlushResult.then( () => continueToolExecutionStart(), (error: unknown) => { - cancelAskUserPromptReservation(); + cancelQuestionPromptReservation(); throw error; }, ); @@ -583,8 +597,8 @@ export function handleToolExecutionStart( } } - if (toolName === "ask_user" && ctx.params.onToolResult) { - const payload = askUserPromptReservation; + if (isQuestionTool && ctx.params.onToolResult) { + const payload = questionPromptReservation; if (payload) { const questionId = payload.questionId; void waitForAskUserPromptReady(questionId) @@ -592,6 +606,22 @@ export function handleToolExecutionStart( if (!questions) { return; } + if (toolName === "secrets") { + const binding = questions[0]?.secretStore; + if (!binding) { + return; + } + const config = ctx.params.config; + const controlUiBase = + resolveControlUiSessionLinkBase(config) ?? + `http://127.0.0.1:${resolveGatewayPort(config)}${normalizeControlUiBasePath( + config?.gateway?.controlUi?.basePath, + )}`; + const url = `${controlUiBase}/ask/${encodeURIComponent(questionId)}`; + return ctx.params.onToolResult?.({ + text: `🔑 Agent requests credential ${binding.name} (${binding.kind}). Reply is disabled for secrets — open to provide it: ${url}`, + }); + } return ctx.params.onToolResult?.( buildAgentHarnessQuestionPromptPayload({ questionId, @@ -607,7 +637,7 @@ export function handleToolExecutionStart( () => settleAskUserPromptDelivery(questionId), (error: unknown) => { settleAskUserPromptDelivery(questionId, error); - ctx.log.warn(`failed to deliver ask_user prompt: ${String(error)}`); + ctx.log.warn(`failed to deliver ${toolName} prompt: ${String(error)}`); }, ); } @@ -622,14 +652,14 @@ export function handleToolExecutionStart( try { flushBlockReplyBufferResult = ctx.flushBlockReplyBuffer(); } catch (error) { - cancelAskUserPromptReservation(); + cancelQuestionPromptReservation(); throw error; } if (isPromiseLike(flushBlockReplyBufferResult)) { return flushBlockReplyBufferResult.then( () => continueAfterBlockReplyFlush(), (error: unknown) => { - cancelAskUserPromptReservation(); + cancelQuestionPromptReservation(); throw error; }, ); diff --git a/src/agents/embedded-agent-subscribe.handlers.tools.test.ts b/src/agents/embedded-agent-subscribe.handlers.tools.test.ts index 47318e577b0d..1b63c923dd83 100644 --- a/src/agents/embedded-agent-subscribe.handlers.tools.test.ts +++ b/src/agents/embedded-agent-subscribe.handlers.tools.test.ts @@ -41,6 +41,7 @@ import { reserveAskUserPromptDelivery, } from "./tools/ask-user-tool.js"; import { resetPendingAskUserQuestionsForTest } from "./tools/ask-user-tool.test-support.js"; +import { createSecretsTool } from "./tools/secrets-tool.js"; type ToolExecutionStartEvent = Omit, "type">; type ToolExecutionEndEvent = Omit, "type">; @@ -438,6 +439,53 @@ describe("handleToolExecutionStart read path checks", () => { await activation.finish(); }); + it("delivers credential requests as absolute Control UI links without answer affordances", async () => { + const { ctx } = createTestContext(); + const onToolResult = vi.fn(); + ctx.params.onToolResult = onToolResult; + ctx.params.config = { + gateway: { + publicOrigin: "https://console.example.test", + controlUi: { basePath: "/control" }, + }, + }; + const args = { action: "request", name: "TEST_API_KEY", kind: "secret" }; + let questionId = ""; + let resolveAnswer: ((value: { status: "cancelled" }) => void) | undefined; + const tool = createSecretsTool({ + agentId: "agent-test-id", + sessionKey: "agent:unit-session", + runId: "run-test", + gatewayCall: async (method, _options, params) => { + if (method === "question.request") { + questionId = String(requireRecord(params, "question request").id); + return { id: questionId }; + } + if (method === "question.get") { + return { question: { questions: [] } }; + } + if (method === "question.waitAnswer") { + return await new Promise((resolve) => { + resolveAnswer = resolve; + }); + } + throw new Error(`unexpected method ${method}`); + }, + }); + + await startTool(ctx, { toolName: "secrets", toolCallId: "secret-call-1", args }); + const pending = tool.execute("secret-call-1", args); + await vi.waitFor(() => expect(onToolResult).toHaveBeenCalledOnce()); + + expect(onToolResult).toHaveBeenCalledWith({ + text: `🔑 Agent requests credential TEST_API_KEY (secret). Reply is disabled for secrets — open to provide it: https://console.example.test/control/ask/${questionId}`, + }); + expect(onToolResult.mock.calls[0]?.[0]).not.toHaveProperty("channelData"); + expect(onToolResult.mock.calls[0]?.[0]).not.toHaveProperty("presentation"); + resolveAnswer?.({ status: "cancelled" }); + await pending; + }); + it.each([ { name: "multi-question", diff --git a/src/agents/openclaw-tools.registration.test.ts b/src/agents/openclaw-tools.registration.test.ts index 69237774d8c8..6868525d678e 100644 --- a/src/agents/openclaw-tools.registration.test.ts +++ b/src/agents/openclaw-tools.registration.test.ts @@ -21,6 +21,7 @@ import { collectPresentOpenClawTools, shouldIncludeAskUserToolForOpenClawTools, shouldIncludeProgressCardToolForOpenClawTools, + shouldIncludeSecretsToolForOpenClawTools, } from "./openclaw-tools.registration.js"; import { textResult, type AnyAgentTool } from "./tools/common.js"; import { createPdfTool } from "./tools/pdf-tool.js"; @@ -119,19 +120,22 @@ describe("openclaw-tools progress_card gating", () => { expect(defaultTools).not.toContain("ask_user"); }); - it("keeps ask_user on primary sessions and excludes spawned worker sessions", () => { - expect(shouldIncludeAskUserToolForOpenClawTools({})).toBe(false); - expect(shouldIncludeAskUserToolForOpenClawTools({ agentSessionKey: "agent:main:main" })).toBe( - true, - ); + it("keeps human-question tools on permitted primary sessions", () => { + for (const includeTool of [ + shouldIncludeAskUserToolForOpenClawTools, + shouldIncludeSecretsToolForOpenClawTools, + ]) { + expect(includeTool({})).toBe(false); + expect(includeTool({ agentSessionKey: "agent:main:main" })).toBe(true); + expect(includeTool({ agentSessionKey: "agent:main:subagent:worker" })).toBe(false); + expect(includeTool({ agentSessionKey: "agent:main:acp:worker" })).toBe(false); + } expect( - shouldIncludeAskUserToolForOpenClawTools({ - agentSessionKey: "agent:main:subagent:worker", + shouldIncludeSecretsToolForOpenClawTools({ + agentSessionKey: "agent:main:main", + pluginToolDenylist: ["secrets"], }), ).toBe(false); - expect( - shouldIncludeAskUserToolForOpenClawTools({ agentSessionKey: "agent:main:acp:worker" }), - ).toBe(false); // ask_user must not depend on the TUI embedded-host flag; normal gateway // runs are the primary consumer. expect( @@ -139,7 +143,7 @@ describe("openclaw-tools progress_card gating", () => { config: {} as OpenClawConfig, runSessionKey: "agent:main:non-embedded", }), - ).toContain("ask_user"); + ).toEqual(expect.arrayContaining(["ask_user", "secrets"])); setEmbeddedMode(true); expect( diff --git a/src/agents/openclaw-tools.registration.ts b/src/agents/openclaw-tools.registration.ts index eca2f4873fa1..33b6f80875f2 100644 --- a/src/agents/openclaw-tools.registration.ts +++ b/src/agents/openclaw-tools.registration.ts @@ -98,12 +98,16 @@ export function shouldIncludeProgressCardToolForOpenClawTools(params: { ); } -/** Includes ask_user only on a primary session and when normal deny policy permits it. */ -export function shouldIncludeAskUserToolForOpenClawTools(params: { +type PrimarySessionToolRegistrationParams = { config?: OpenClawConfig; agentSessionKey?: string; pluginToolDenylist?: string[]; -}): boolean { +}; + +function shouldIncludePrimarySessionToolForOpenClawTools( + toolName: "ask_user" | "secrets", + params: PrimarySessionToolRegistrationParams, +): boolean { const sessionKey = params.agentSessionKey?.trim(); if (!sessionKey) { return false; @@ -112,5 +116,19 @@ export function shouldIncludeAskUserToolForOpenClawTools(params: { ...(params.config?.tools?.deny ?? []), ...(params.pluginToolDenylist ?? []), ]); - return isPrimaryBootstrapRun(sessionKey) && isToolAllowedByPolicyName("ask_user", { deny }); + return isPrimaryBootstrapRun(sessionKey) && isToolAllowedByPolicyName(toolName, { deny }); +} + +/** Includes ask_user only on a primary session and when normal deny policy permits it. */ +export function shouldIncludeAskUserToolForOpenClawTools( + params: PrimarySessionToolRegistrationParams, +): boolean { + return shouldIncludePrimarySessionToolForOpenClawTools("ask_user", params); +} + +/** Keeps credential management on primary sessions allowed by the normal tool policy. */ +export function shouldIncludeSecretsToolForOpenClawTools( + params: PrimarySessionToolRegistrationParams, +): boolean { + return shouldIncludePrimarySessionToolForOpenClawTools("secrets", params); } diff --git a/src/agents/openclaw-tools.ts b/src/agents/openclaw-tools.ts index 6fbb3cb0e33b..b960cc9257a8 100644 --- a/src/agents/openclaw-tools.ts +++ b/src/agents/openclaw-tools.ts @@ -27,6 +27,7 @@ import { collectPresentOpenClawTools, shouldIncludeAskUserToolForOpenClawTools, shouldIncludeProgressCardToolForOpenClawTools, + shouldIncludeSecretsToolForOpenClawTools, } from "./openclaw-tools.registration.js"; import { createRequesterYieldCallback } from "./openclaw-tools.requester-yield.js"; import { createOpenClawSwarmToolGroups } from "./openclaw-tools.swarm.js"; @@ -68,6 +69,7 @@ import { createPdfTool } from "./tools/pdf-tool.js"; import { createPortalTool } from "./tools/portal-tool.js"; import { createProgressCardTool } from "./tools/progress-card-tool.js"; import { createScreenTool } from "./tools/screen-tool.js"; +import { createSecretsTool } from "./tools/secrets-tool.js"; import { createSessionStatusTool } from "./tools/session-status-tool.js"; import { createSessionsHistoryTool } from "./tools/sessions-history-tool.js"; import { createSessionsListTool } from "./tools/sessions-list-tool.js"; @@ -478,6 +480,19 @@ export function createOpenClawTools(options?: OpenClawToolsOptions): AnyAgentToo }), ] : []), + ...(shouldIncludeSecretsToolForOpenClawTools({ + config: resolvedConfig, + agentSessionKey: options?.runSessionKey ?? options?.agentSessionKey, + pluginToolDenylist: options?.pluginToolDenylist, + }) + ? [ + createSecretsTool({ + agentId: sessionAgentId, + sessionKey: options?.runSessionKey ?? options?.agentSessionKey, + runId: options?.runId, + }), + ] + : []), createSessionsListTool({ ...sessionLookupToolOptions, requesterAgentIdOverride: sessionAgentId, diff --git a/src/agents/tool-catalog.test.ts b/src/agents/tool-catalog.test.ts index 7ea4156ef777..482a31ecf9d2 100644 --- a/src/agents/tool-catalog.test.ts +++ b/src/agents/tool-catalog.test.ts @@ -50,6 +50,7 @@ describe("tool-catalog", () => { "exec", "process", "code_execution", + "secrets", "web_search", "web_fetch", "x_search", @@ -94,6 +95,7 @@ describe("tool-catalog", () => { it("includes bundle MCP tools in coding and messaging profile policies", () => { expect(requirePolicyAllow("coding").at(-1)).toBe("bundle-mcp"); expect(requirePolicyAllow("messaging")).toEqual([ + "secrets", "sessions", "sessions_list", "sessions_history", diff --git a/src/agents/tool-catalog.ts b/src/agents/tool-catalog.ts index 7e7fccaccf15..0033187c02e9 100644 --- a/src/agents/tool-catalog.ts +++ b/src/agents/tool-catalog.ts @@ -118,6 +118,14 @@ const CORE_TOOL_DEFINITIONS: CoreToolDefinition[] = [ profiles: ["coding"], includeInOpenClawGroup: true, }, + { + id: "secrets", + label: "secrets", + description: "Request and manage write-only credentials", + sectionId: "runtime", + profiles: ["coding", "messaging"], + includeInOpenClawGroup: true, + }, { id: "web_search", label: "web_search", diff --git a/src/agents/tool-description-presets.ts b/src/agents/tool-description-presets.ts index c0af230be5a9..fbb06d8bf8b3 100644 --- a/src/agents/tool-description-presets.ts +++ b/src/agents/tool-description-presets.ts @@ -166,3 +166,15 @@ export function describeAskUserTool(): string { "If the result is no_answer, continue with best judgment.", ].join(" "); } + +/** Describes the secrets tool and the store semantics the model cannot observe. */ +export function describeSecretsTool(): string { + return [ + "Obtain and manage credentials you never see: `request` asks the human to type a value into a trusted prompt that stores it directly, `list` returns entry metadata, and `delete` removes an entry.", + "A requested value is never readable back by any action; use `request` when you need a credential you do not have instead of asking for one in conversation, and never repeat a credential a human pasted into chat.", + "`request` blocks until the human answers, so ask only for a credential the current task actually needs.", + "Only protected secrets may be requested, and they reach a service through config references or, where the egress proxy is enabled, substitution into outbound requests; plain environment values are set by the operator in Settings or the CLI, never requested here.", + "List every hostname that will receive the value in `allowedHosts`: a secret with no allowed hosts can never be substituted, so the request silently produces an unusable credential.", + '`reason` is shown to the human deciding whether to provide the value. Stored entries are referenced elsewhere as {source:"store", id:NAME}; if the result is no_answer, continue with best judgment.', + ].join(" "); +} diff --git a/src/agents/tool-display-config.ts b/src/agents/tool-display-config.ts index 05a3433487aa..468813df6239 100644 --- a/src/agents/tool-display-config.ts +++ b/src/agents/tool-display-config.ts @@ -225,6 +225,11 @@ export const TOOL_DISPLAY_CONFIG: ToolDisplayConfig = { title: "Ask User", detailKeys: ["questions.0.question"], }, + secrets: { + emoji: "🔑", + title: "Secrets", + detailKeys: ["action", "name", "kind"], + }, suggest_task: { emoji: "✨", title: "Suggest Task", diff --git a/src/agents/tools/ask-user-tool.ts b/src/agents/tools/ask-user-tool.ts index 75e2397b458d..86bf56c796fd 100644 --- a/src/agents/tools/ask-user-tool.ts +++ b/src/agents/tools/ask-user-tool.ts @@ -16,14 +16,14 @@ import { normalizeAskUserParams, } from "./ask-user-tool-normalization.js"; import { type AnyAgentTool, ToolInputError, textResult } from "./common.js"; +import { + createGatewayQuestionCanceller, + readQuestionErrorReason, +} from "./gateway-question-lifecycle.js"; import { callGatewayTool, type GatewayCallOptions } from "./gateway.js"; const ASK_USER_RPC_GRACE_MS = 10_000; const ASK_USER_PROMPT_RECHECK_MS = 50; -const TERMINAL_QUESTION_ERROR_REASONS = new Set([ - "QUESTION_ALREADY_TERMINAL", - "QUESTION_NOT_FOUND", -]); const AskUserToolSchema = Type.Object( { @@ -85,7 +85,6 @@ type AskUserQuestionState = { questions: QuestionRequestQuestion[]; expiresAtMs: number; phase: AskUserQuestionPhase; - gatewayCall?: AskUserGatewayCall; answer?: Promise; claim?: ReturnType; waiters: Set<() => void>; @@ -423,25 +422,60 @@ async function waitForPromptDelivery( return { error: new Error("ask_user prompt is no longer active") }; } -function readQuestionErrorReason(error: unknown): string | undefined { - if (!error || typeof error !== "object") { - return undefined; +/** Shares question ownership and prompt delivery without installing a plaintext answer claim. */ +export function beginAskUserPromptDelivery(params: { + toolCallId: string; + sessionKey?: string; + runId?: string; + agentId?: string; + questions: QuestionRequestQuestion[]; + timeoutSeconds: number; +}) { + const questionId = buildAskUserQuestionId( + params.toolCallId, + params.sessionKey, + params.runId, + params.agentId, + ); + const sessionKey = askUserSessionKey(params.sessionKey, params.agentId); + const reserved = askUserQuestions.get(questionId); + const existing = findAskUserQuestionForSession(sessionKey); + if ((reserved && reserved.phase.kind !== "reserved") || (existing && existing !== reserved)) { + throw new ToolInputError( + "a question is already pending for this session; wait for it to resolve before requesting another", + ); } - const requestError = error as { details?: unknown; name?: unknown }; - if (requestError.name !== "GatewayClientRequestError") { - return undefined; - } - const details = requestError.details; - if (!details || typeof details !== "object" || Array.isArray(details)) { - return undefined; - } - const reason = (details as { reason?: unknown }).reason; - return typeof reason === "string" ? reason : undefined; -} - -function isTerminalQuestionResolveError(error: unknown): boolean { - const reason = readQuestionErrorReason(error); - return reason !== undefined && TERMINAL_QUESTION_ERROR_REASONS.has(reason); + const state: AskUserQuestionState = reserved ?? { + questionId, + sessionKey, + questions: params.questions, + expiresAtMs: 0, + phase: { kind: "registering" }, + waiters: new Set(), + }; + Object.assign(state, { sessionKey, questions: params.questions }); + state.expiresAtMs = Date.now() + params.timeoutSeconds * 1_000; + transitionAskUserQuestion(state, { kind: "registering" }); + askUserQuestions.set(questionId, state); + return { + questionId, + hasSubscriber: reserved !== undefined, + markReady() { + if (reserved) { + markAskUserPromptReady(questionId, params.questions); + } else { + transitionAskUserQuestion(state, { kind: "answerable" }); + } + }, + waitForDelivery(signal?: AbortSignal) { + return waitForPromptDelivery(state, signal); + }, + release() { + if (askUserQuestions.get(questionId) === state) { + releaseAskUserQuestion(questionId); + } + }, + }; } function resetPendingAskUserQuestionsForTest(): void { @@ -504,44 +538,14 @@ export function createAskUserTool(params: { questions: normalized.questions, expiresAtMs: Date.now() + timeoutMs, phase: { kind: "registering" }, - gatewayCall, waiters: new Set(), } satisfies AskUserQuestionState); - Object.assign(state, { sessionKey, questions: normalized.questions, gatewayCall }); + Object.assign(state, { sessionKey, questions: normalized.questions }); state.expiresAtMs = Date.now() + timeoutMs; transitionAskUserQuestion(state, { kind: "registering" }); askUserQuestions.set(questionId, state); - let cancellation: - | Promise | undefined> - | undefined; let registered = false; - const cancelPendingQuestion = (resolvedBy: string) => { - cancellation ??= (async () => { - try { - await gatewayCall( - "question.resolve", - { timeoutMs: ASK_USER_RPC_GRACE_MS }, - { id: questionId, cancel: true, resolvedBy }, - ); - return undefined; - } catch (error) { - if (!isTerminalQuestionResolveError(error)) { - return undefined; - } - try { - const result = (await gatewayCall( - "question.waitAnswer", - { timeoutMs: ASK_USER_RPC_GRACE_MS }, - { id: questionId, timeoutMs: 1_000 }, - )) as QuestionWaitAnswerResult; - return result.status === "answered" ? result : undefined; - } catch { - return undefined; - } - } - })(); - return cancellation; - }; + const cancelPendingQuestion = createGatewayQuestionCanceller({ gatewayCall, questionId }); const cancelOnAbort = () => { if (askUserQuestions.get(questionId) === state) { releaseAskUserQuestion(questionId); diff --git a/src/agents/tools/gateway-question-lifecycle.ts b/src/agents/tools/gateway-question-lifecycle.ts new file mode 100644 index 000000000000..5eaa5c56346e --- /dev/null +++ b/src/agents/tools/gateway-question-lifecycle.ts @@ -0,0 +1,107 @@ +/** Shared registration, wait, and cancellation for blocking Gateway questions. */ +import { asNullableRecord } from "@openclaw/normalization-core/record-coerce"; +import { Value } from "typebox/value"; +import { + QuestionWaitAnswerResultSchema, + type QuestionWaitAnswerResult, +} from "../../../packages/gateway-protocol/src/index.js"; +import type { OperatorScope } from "../../gateway/operator-scopes.js"; +import type { GatewayCallOptions } from "./gateway.js"; + +/** Grace added to Gateway RPC deadlines so the question's own timeout wins. */ +const QUESTION_RPC_GRACE_MS = 10_000; + +export type GatewayQuestionCall = ( + method: string, + opts: GatewayCallOptions, + params?: unknown, + // Mirrors callGatewayTool's extra bag so every question caller shares one type. + extra?: { + expectFinal?: boolean; + scopes?: OperatorScope[]; + requireAgentRuntimeIdentity?: boolean; + signal?: AbortSignal; + }, +) => Promise; + +const TERMINAL_QUESTION_ERROR_REASONS = new Set([ + "QUESTION_ALREADY_TERMINAL", + "QUESTION_NOT_FOUND", +]); + +/** Reads the Gateway's structured failure reason from a question RPC rejection. */ +export function readQuestionErrorReason(error: unknown): string | undefined { + const requestError = asNullableRecord(error); + if (requestError?.name !== "GatewayClientRequestError") { + return undefined; + } + const reason = asNullableRecord(requestError.details)?.reason; + return typeof reason === "string" ? reason : undefined; +} + +function isTerminalQuestionResolveError(error: unknown): boolean { + const reason = readQuestionErrorReason(error); + return reason !== undefined && TERMINAL_QUESTION_ERROR_REASONS.has(reason); +} + +/** Waits for one question's terminal state, validating the Gateway's payload. */ +export async function awaitGatewayQuestionAnswer(params: { + gatewayCall: GatewayQuestionCall; + questionId: string; + timeoutMs: number; + signal?: AbortSignal; +}): Promise { + const result = await params.gatewayCall( + "question.waitAnswer", + { timeoutMs: params.timeoutMs + QUESTION_RPC_GRACE_MS }, + { id: params.questionId, timeoutMs: params.timeoutMs }, + params.signal ? { signal: params.signal } : undefined, + ); + if (!Value.Check(QuestionWaitAnswerResultSchema, result)) { + throw new Error("question.waitAnswer returned an invalid status"); + } + return result; +} + +/** + * Cancels a pending question at most once. An answer that lands between the + * caller's timeout and this cancel makes the Gateway reject it as terminal; the + * recovery read returns that answer so a submitted response is never discarded. + */ +export function createGatewayQuestionCanceller(params: { + gatewayCall: GatewayQuestionCall; + questionId: string; +}): ( + resolvedBy: string, +) => Promise | undefined> { + let cancellation: + | Promise | undefined> + | undefined; + return (resolvedBy: string) => { + cancellation ??= (async () => { + try { + await params.gatewayCall( + "question.resolve", + { timeoutMs: QUESTION_RPC_GRACE_MS }, + { id: params.questionId, cancel: true, resolvedBy }, + ); + return undefined; + } catch (error) { + if (!isTerminalQuestionResolveError(error)) { + return undefined; + } + try { + const result = await awaitGatewayQuestionAnswer({ + gatewayCall: params.gatewayCall, + questionId: params.questionId, + timeoutMs: 1_000, + }); + return result.status === "answered" ? result : undefined; + } catch { + return undefined; + } + } + })(); + return cancellation; + }; +} diff --git a/src/agents/tools/gateway.runtime-identity.test.ts b/src/agents/tools/gateway.runtime-identity.test.ts index df3f60d5a4d4..568a120dce6f 100644 --- a/src/agents/tools/gateway.runtime-identity.test.ts +++ b/src/agents/tools/gateway.runtime-identity.test.ts @@ -468,6 +468,7 @@ describe("gateway tool runtime identity", () => { it.each([ ["exec.approval.request", undefined, false], ["plugin.approval.request", "codex", false], + ["secrets.store.delete", undefined, false], ["exec.approval.request", undefined, true], ["plugin.approval.request", "codex", true], ] as const)( diff --git a/src/agents/tools/gateway.ts b/src/agents/tools/gateway.ts index 8d36ff2ef42e..f22fb523668d 100644 --- a/src/agents/tools/gateway.ts +++ b/src/agents/tools/gateway.ts @@ -231,6 +231,7 @@ const AGENT_RUNTIME_IDENTITY_METHODS = new Set([ "cron.remove", "cron.run", "cron.runs", + "secrets.store.delete", ]); const OPTIONAL_LOCAL_AGENT_RUNTIME_IDENTITY_METHODS = new Set(["node.invoke"]); diff --git a/src/agents/tools/secrets-tool.test.ts b/src/agents/tools/secrets-tool.test.ts new file mode 100644 index 000000000000..039ac15d058c --- /dev/null +++ b/src/agents/tools/secrets-tool.test.ts @@ -0,0 +1,426 @@ +import { Value } from "typebox/value"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { claimPendingAgentQuestionAnswer } from "../harness/gateway-question.js"; +import { reserveAskUserPromptDelivery, settleAskUserPromptDelivery } from "./ask-user-tool.js"; +import { resetPendingAskUserQuestionsForTest } from "./ask-user-tool.test-support.js"; +import { createSecretsTool, normalizeSecretsRequestParams } from "./secrets-tool.js"; + +type GatewayCall = NonNullable[0]["gatewayCall"]>; + +function gatewayStub( + implementation: ( + method: string, + opts: Record, + params: Record, + extra?: { signal?: AbortSignal; requireAgentRuntimeIdentity?: boolean }, + ) => Promise, +) { + const mock = vi.fn(implementation); + return { mock, call: mock as unknown as GatewayCall }; +} + +function requestedQuestionId(mock: ReturnType["mock"]): string { + const request = mock.mock.calls.find(([method]) => method === "question.request"); + const questionId = request?.[2].id; + if (typeof questionId !== "string") { + throw new Error("question.request did not include an id"); + } + return questionId; +} + +afterEach(() => { + resetPendingAskUserQuestionsForTest(); +}); + +describe("secrets request normalization", () => { + it("builds one store-bound secret question and clamps its timeout", () => { + const normalized = normalizeSecretsRequestParams({ + action: "request", + name: "SERVICE_API_KEY", + kind: "secret", + allowedHosts: ["api.example.test"], + reason: "Deploy the service", + timeoutSeconds: 5, + }); + + expect(normalized).toEqual({ + name: "SERVICE_API_KEY", + kind: "secret", + allowedHosts: ["api.example.test"], + reason: "Deploy the service", + timeoutSeconds: 30, + questions: [ + { + questionId: "secret_value", + header: "API key", + question: "Provide the secret for SERVICE_API_KEY. Deploy the service", + options: [], + isSecret: true, + secretStore: { + name: "SERVICE_API_KEY", + kind: "secret", + allowedHosts: ["api.example.test"], + reason: "Deploy the service", + }, + }, + ], + }); + expect( + normalizeSecretsRequestParams({ + name: "SERVICE_SETTING", + kind: "secret", + timeoutSeconds: 9_999, + }).timeoutSeconds, + ).toBe(3_600); + expect( + Value.Check(createSecretsTool({}).parameters, { + action: "set", + name: "SERVICE_API_KEY", + value: "test-secret-value-123", + }), + ).toBe(false); + }); + + it.each([ + ["lowercase names", { name: "bad_name", kind: "secret" }, "uppercase"], + ["unknown entry kinds", { name: "VALID_NAME", kind: "password" }, "kind must be"], + [ + "environment-value requests the model could read back", + { name: "VALID_NAME", kind: "env" }, + 'kind must be "secret"', + ], + [ + "duplicate allowed hosts", + { name: "VALID_NAME", kind: "secret", allowedHosts: ["a.test", "a.test"] }, + "unique", + ], + [ + "oversized reasons", + { name: "VALID_NAME", kind: "secret", reason: "r".repeat(201) }, + "at most 200", + ], + ["fractional timeouts", { name: "VALID_NAME", kind: "secret", timeoutSeconds: 1.5 }, "integer"], + ])("rejects %s before contacting the gateway", (_label, params, message) => { + expect(() => normalizeSecretsRequestParams(params)).toThrow(message); + }); +}); + +describe("secrets tool", () => { + it("stores through a human-only question and returns metadata without claiming chat text", async () => { + let finishWait: ((value: unknown) => void) | undefined; + const gateway = gatewayStub(async (method, _options, params) => { + if (method === "question.request") { + return { id: params.id }; + } + if (method === "question.get") { + return { question: { questions: [{ secretStoreExisting: { updatedAtMs: 123 } }] } }; + } + if (method === "question.waitAnswer") { + return await new Promise((resolve) => { + finishWait = resolve; + }); + } + throw new Error(`unexpected method ${method}`); + }); + const tool = createSecretsTool({ + agentId: "main", + sessionKey: "agent:main:secrets", + runId: "run-secrets", + gatewayCall: gateway.call, + }); + const pending = tool.execute("call-secret", { + action: "request", + name: "SERVICE_API_KEY", + kind: "secret", + allowedHosts: ["api.example.test"], + reason: "Deploy the service", + }); + await vi.waitFor(() => expect(finishWait).toBeTypeOf("function")); + + await expect( + claimPendingAgentQuestionAnswer({ + sessionKey: "agent:main:secrets", + text: "test-secret-value-123", + }), + ).resolves.toBe(false); + finishWait?.({ + status: "answered", + answers: { answers: { secret_value: ["stored"] } }, + }); + const result = await pending; + + expect(result.details).toEqual({ + status: "stored", + name: "SERVICE_API_KEY", + kind: "secret", + allowedHosts: ["api.example.test"], + replacedExisting: true, + ref: { source: "store", id: "SERVICE_API_KEY" }, + }); + expect(JSON.stringify(result)).not.toContain("test-secret-value-123"); + expect(result.content[0]).toMatchObject({ + text: expect.stringContaining('{source:"store", id:"SERVICE_API_KEY"}'), + }); + expect(gateway.mock).toHaveBeenCalledWith( + "question.request", + {}, + expect.objectContaining({ + id: expect.stringMatching(/^ask_[a-f0-9]{32}$/), + agentId: "main", + sessionKey: "agent:main:secrets", + runId: "run-secrets", + timeoutMs: 900_000, + questions: [ + expect.objectContaining({ + questionId: "secret_value", + options: [], + isSecret: true, + secretStore: { + name: "SERVICE_API_KEY", + kind: "secret", + allowedHosts: ["api.example.test"], + reason: "Deploy the service", + }, + }), + ], + }), + // Store-bound minting is admin-gated server-side; the tool must declare + // the scope explicitly instead of the questions-scope default. + { scopes: ["operator.admin"] }, + ); + }); + + it("continues a registered credential request when optional replacement metadata is unavailable", async () => { + const gateway = gatewayStub(async (method, _options, params) => { + if (method === "question.request") { + return { id: params.id }; + } + if (method === "question.get") { + throw new Error("metadata temporarily unavailable"); + } + return { status: "answered", answers: { answers: { secret_value: ["stored"] } } }; + }); + + const result = await createSecretsTool({ gatewayCall: gateway.call }).execute("call-metadata", { + action: "request", + name: "SERVICE_SETTING", + kind: "secret", + }); + + expect(result.details).toMatchObject({ + status: "stored", + kind: "secret", + replacedExisting: false, + }); + expect(gateway.mock.mock.calls.some(([method]) => method === "question.resolve")).toBe(false); + }); + + it.each(["pending", "expired", "cancelled"] as const)( + "returns no_answer when a credential request is %s", + async (status) => { + const gateway = gatewayStub(async (method, _options, params) => { + if (method === "question.request") { + return { id: params.id }; + } + if (method === "question.get") { + return { question: { questions: [{}] } }; + } + return { status }; + }); + + const result = await createSecretsTool({ + sessionKey: `agent:main:${status}`, + gatewayCall: gateway.call, + }).execute(`call-${status}`, { action: "request", name: "SERVICE_API_KEY", kind: "secret" }); + + expect(result.details).toEqual({ status: "no_answer" }); + if (status === "pending") { + expect(gateway.mock).toHaveBeenCalledWith( + "question.resolve", + { timeoutMs: 10_000 }, + { + id: requestedQuestionId(gateway.mock), + cancel: true, + resolvedBy: "wait-timeout", + }, + ); + } + }, + ); + + it("keeps a credential stored when the human answers during the wait timeout", async () => { + // The Gateway rejects the late cancel as terminal and hands back the answer; + // the value is already in the store, so the tool must not report no_answer. + const terminal = Object.assign(new Error("question is already answered"), { + name: "GatewayClientRequestError", + details: { reason: "QUESTION_ALREADY_TERMINAL" }, + }); + let waitCalls = 0; + const gateway = gatewayStub(async (method, _options, params) => { + if (method === "question.request") { + return { id: params.id }; + } + if (method === "question.get") { + return { question: { questions: [{}] } }; + } + if (method === "question.resolve") { + throw terminal; + } + waitCalls += 1; + return waitCalls === 1 + ? { status: "pending" } + : { status: "answered", answers: { answers: { secret_value: ["stored"] } } }; + }); + + const result = await createSecretsTool({ + sessionKey: "agent:main:late-answer", + gatewayCall: gateway.call, + }).execute("call-late-answer", { + action: "request", + name: "SERVICE_API_KEY", + kind: "secret", + }); + + expect(result.details).toMatchObject({ status: "stored", name: "SERVICE_API_KEY" }); + }); + + it("cancels a registered credential request when its agent run aborts", async () => { + const controller = new AbortController(); + const gateway = gatewayStub(async (method, _options, params, extra) => { + if (method === "question.request") { + return { id: params.id }; + } + if (method === "question.get") { + return { question: { questions: [{}] } }; + } + if (method === "question.resolve") { + return { status: "cancelled" }; + } + return await new Promise((_resolve, reject) => { + extra?.signal?.addEventListener("abort", () => reject(new Error("aborted")), { + once: true, + }); + }); + }); + const pending = createSecretsTool({ + sessionKey: "agent:main:secret-abort", + gatewayCall: gateway.call, + }).execute( + "call-secret-abort", + { action: "request", name: "SERVICE_API_KEY", kind: "secret" }, + controller.signal, + ); + await vi.waitFor(() => + expect(gateway.mock.mock.calls.some(([method]) => method === "question.waitAnswer")).toBe( + true, + ), + ); + + controller.abort(new Error("stop")); + + await expect(pending).rejects.toThrow("aborted"); + expect(gateway.mock).toHaveBeenCalledWith( + "question.resolve", + { timeoutMs: 10_000 }, + { id: requestedQuestionId(gateway.mock), cancel: true, resolvedBy: "run-abort" }, + ); + }); + + it("shares the existing subscriber prompt reservation and settlement lifecycle", async () => { + const sessionKey = "agent:main:secret-prompt"; + const args = { action: "request", name: "SERVICE_API_KEY", kind: "secret" }; + const normalized = normalizeSecretsRequestParams(args); + const reservation = reserveAskUserPromptDelivery({ + toolCallId: "call-secret-prompt", + sessionKey, + questions: normalized.questions, + timeoutSeconds: normalized.timeoutSeconds, + }); + if (!reservation) { + throw new Error("expected secret prompt reservation"); + } + let finishWait: ((value: unknown) => void) | undefined; + const gateway = gatewayStub(async (method, _options, params) => { + if (method === "question.request") { + return { id: params.id }; + } + if (method === "question.get") { + return { question: { questions: [{}] } }; + } + if (method === "question.waitAnswer") { + return await new Promise((resolve) => { + finishWait = resolve; + }); + } + throw new Error(`unexpected method ${method}`); + }); + const pending = createSecretsTool({ sessionKey, gatewayCall: gateway.call }).execute( + "call-secret-prompt", + args, + ); + await vi.waitFor(() => expect(finishWait).toBeTypeOf("function")); + + settleAskUserPromptDelivery(reservation.questionId); + finishWait?.({ + status: "answered", + answers: { answers: { secret_value: ["stored"] } }, + }); + + await expect(pending).resolves.toMatchObject({ details: { status: "stored" } }); + }); + + it("lists store metadata and environment previews", async () => { + const entries = [ + { + name: "SERVICE_API_KEY", + kind: "secret", + allowedHosts: ["api.example.test"], + createdAtMs: 0, + updatedAtMs: 0, + updatedBy: "operator:alice", + scopeKind: "team", + scopeId: "", + }, + { + name: "SERVICE_MODE", + kind: "env", + value: "preview-value", + createdAtMs: 0, + updatedAtMs: 0, + scopeKind: "team", + scopeId: "", + }, + ]; + const gateway = gatewayStub(async () => ({ entries })); + + const result = await createSecretsTool({ gatewayCall: gateway.call }).execute("call-list", { + action: "list", + }); + + expect(result.details).toEqual({ entries }); + expect(result.content[0]).toMatchObject({ + text: expect.stringContaining("SERVICE_API_KEY | secret | hosts: api.example.test"), + }); + expect(result.content[0]).toMatchObject({ + text: expect.stringContaining("value: preview-value"), + }); + expect(gateway.mock).toHaveBeenCalledWith("secrets.store.list", {}, {}, undefined); + }); + + it("requires verified agent runtime identity when deleting a store entry", async () => { + const gateway = gatewayStub(async () => ({ ok: true, reloaded: false })); + + const result = await createSecretsTool({ gatewayCall: gateway.call }).execute("call-delete", { + action: "delete", + name: "SERVICE_API_KEY", + }); + + expect(result.details).toEqual({ ok: true, reloaded: false }); + expect(gateway.mock).toHaveBeenCalledWith( + "secrets.store.delete", + {}, + { name: "SERVICE_API_KEY" }, + { requireAgentRuntimeIdentity: true }, + ); + }); +}); diff --git a/src/agents/tools/secrets-tool.ts b/src/agents/tools/secrets-tool.ts new file mode 100644 index 000000000000..4f3eafb8c6d3 --- /dev/null +++ b/src/agents/tools/secrets-tool.ts @@ -0,0 +1,352 @@ +import { asNullableRecord, isRecord } from "@openclaw/normalization-core/record-coerce"; +import { Type } from "typebox"; +import { + validateSecretsStoreListResult, + type QuestionRequestQuestion, + type SecretsStoreListResult, +} from "../../../packages/gateway-protocol/src/index.js"; +import { ENV_SECRET_REF_ID_RE } from "../../config/types.secrets.js"; +import { ADMIN_SCOPE } from "../../gateway/operator-scopes.js"; +import { stringEnum } from "../schema/string-enum.js"; +import { describeSecretsTool } from "../tool-description-presets.js"; +import { DEFAULT_ASK_USER_TIMEOUT_SECONDS } from "./ask-user-tool-normalization.js"; +import { beginAskUserPromptDelivery } from "./ask-user-tool.js"; +import { type AnyAgentTool, readToolStringParam, ToolInputError } from "./common.js"; +import { + awaitGatewayQuestionAnswer, + createGatewayQuestionCanceller, + type GatewayQuestionCall, +} from "./gateway-question-lifecycle.js"; +import { callGatewayTool } from "./gateway.js"; +import { jsonResult, textResult } from "./tool-results.js"; + +type SecretStoreKind = "secret"; +const SecretsToolSchema = Type.Object( + { + action: stringEnum(["request", "list", "delete"], { + description: "`request` a value from the human, `list` entry metadata, or `delete` an entry.", + }), + name: Type.Optional( + Type.String({ + maxLength: 128, + pattern: "^[A-Z][A-Z0-9_]{0,127}$", + description: + "Entry name in uppercase environment-variable form, also its SecretRef id (STRIPE_API_KEY). Required for request and delete.", + }), + ), + kind: Type.Optional( + stringEnum(["secret"], { + description: "Only `secret` may be requested; requested values are never readable back.", + }), + ), + allowedHosts: Type.Optional( + Type.Array(Type.String({ minLength: 1, maxLength: 253 }), { + maxItems: 128, + uniqueItems: true, + description: + "Exact hostnames allowed to receive a secret, without scheme or port (api.stripe.com). Secret entries only; leaving this empty stores a secret that can never be substituted.", + }), + ), + reason: Type.Optional( + Type.String({ + maxLength: 200, + description: "One line shown to the human explaining why the credential is needed.", + }), + ), + timeoutSeconds: Type.Optional( + Type.Integer({ + description: "Seconds to wait for the human on request; defaults to 900, clamped 30-3600.", + }), + ), + }, + { additionalProperties: false }, +); + +type NormalizedSecretsRequestParams = { + name: string; + kind: SecretStoreKind; + allowedHosts?: string[]; + reason?: string; + timeoutSeconds: number; + questions: QuestionRequestQuestion[]; +}; + +function readSecretStoreName(params: Record): string { + const name = readToolStringParam(params, "name", { required: true }); + if (!ENV_SECRET_REF_ID_RE.test(name)) { + throw new ToolInputError("name must be an uppercase environment-variable name"); + } + return name; +} + +/** Normalizes one secure question for both tool-start reservation and tool execution. */ +export function normalizeSecretsRequestParams(value: unknown): NormalizedSecretsRequestParams { + if (!isRecord(value)) { + throw new ToolInputError("secrets arguments must be an object"); + } + const params = value; + const name = readSecretStoreName(params); + // Requests are secret-only on purpose: `list` renders env values, so an + // agent-requested env entry would be readable straight back through this + // tool, breaking the promise the masked prompt makes to the human. + const kind = readToolStringParam(params, "kind", { required: false }) ?? "secret"; + if (kind !== "secret") { + throw new ToolInputError( + 'kind must be "secret"; environment values are set in Settings or the CLI, not requested from the model', + ); + } + const allowedHosts = params.allowedHosts; + if (allowedHosts !== undefined) { + if ( + !Array.isArray(allowedHosts) || + allowedHosts.length > 128 || + allowedHosts.some((host) => typeof host !== "string" || !host || host.length > 253) || + new Set(allowedHosts).size !== allowedHosts.length + ) { + throw new ToolInputError("allowedHosts must contain up to 128 unique non-empty hostnames"); + } + } + if (params.reason !== undefined && typeof params.reason !== "string") { + throw new ToolInputError("reason must be a string"); + } + const reason = typeof params.reason === "string" ? params.reason.trim() : undefined; + if (reason && reason.length > 200) { + throw new ToolInputError("reason must be at most 200 characters"); + } + const timeout = params.timeoutSeconds; + if ( + timeout !== undefined && + (typeof timeout !== "number" || !Number.isFinite(timeout) || !Number.isInteger(timeout)) + ) { + throw new ToolInputError("timeoutSeconds must be an integer"); + } + const timeoutSeconds = Math.min(3_600, Math.max(30, timeout ?? DEFAULT_ASK_USER_TIMEOUT_SECONDS)); + const binding: NonNullable = { + name, + kind: "secret", + ...(allowedHosts !== undefined ? { allowedHosts } : {}), + ...(reason ? { reason } : {}), + }; + const question = `Provide the secret for ${name}.${reason ? ` ${reason}` : ""}`; + return { + ...binding, + kind: "secret", + timeoutSeconds, + questions: [ + { + questionId: "secret_value", + header: "API key", + question, + options: [], + isSecret: true, + secretStore: binding, + }, + ], + }; +} + +function noSecretAnswerResult(status: "pending" | "expired" | "cancelled") { + const details = { status: "no_answer" as const }; + const note = + status === "cancelled" + ? "The credential request was cancelled; proceed with best judgment." + : "No credential arrived; proceed with best judgment."; + return textResult(`${note}\n\n${JSON.stringify(details, null, 2)}`, details); +} + +function storedSecretResult(params: NormalizedSecretsRequestParams, replacedExisting: boolean) { + const details = { + status: "stored" as const, + name: params.name, + kind: params.kind, + ...(params.allowedHosts !== undefined ? { allowedHosts: params.allowedHosts } : {}), + replacedExisting, + ref: { source: "store" as const, id: params.name }, + }; + const guidance = [ + `Stored ${params.name} without exposing its value.`, + `Reference {source:"store", id:"${params.name}"} in config SecretRefs.`, + "Secret values are substituted at egress only when secrets.egressProxy.enabled is true and the destination matches their allowed hosts.", + ]; + return textResult(`${guidance.join(" ")}\n\n${JSON.stringify(details, null, 2)}`, details); +} + +function listSecretStoreResult(result: SecretsStoreListResult) { + const lines = result.entries.map((entry) => { + const fields = [entry.name, entry.kind]; + if (entry.kind === "secret" && entry.allowedHosts?.length) { + fields.push(`hosts: ${entry.allowedHosts.join(", ")}`); + } + if (entry.kind === "env") { + fields.push(`value: ${entry.value}`); + } + fields.push(`updated: ${new Date(entry.updatedAtMs).toISOString()}`); + if (entry.updatedBy) { + fields.push(`by: ${entry.updatedBy}`); + } + return fields.join(" | "); + }); + return textResult(lines.length ? lines.join("\n") : "The secret store is empty.", result); +} + +/** Creates the metadata-only secret-store tool and its human-entered write flow. */ +export function createSecretsTool(params: { + agentId?: string; + sessionKey?: string; + runId?: string; + gatewayCall?: GatewayQuestionCall; +}): AnyAgentTool { + const gatewayCall: GatewayQuestionCall = params.gatewayCall ?? callGatewayTool; + return { + label: "Secrets", + name: "secrets", + description: describeSecretsTool(), + parameters: SecretsToolSchema, + execute: async (toolCallId, args, signal) => { + if (!isRecord(args)) { + throw new ToolInputError("secrets arguments must be an object"); + } + const input = args; + const action = readToolStringParam(input, "action", { required: true }); + if (action === "list") { + const result = await gatewayCall( + "secrets.store.list", + {}, + {}, + signal ? { signal } : undefined, + ); + if (!validateSecretsStoreListResult(result)) { + throw new Error("secrets.store.list returned invalid metadata"); + } + return listSecretStoreResult(result); + } + if (action === "delete") { + const name = readSecretStoreName(input); + const result = await gatewayCall( + "secrets.store.delete", + {}, + { name }, + { requireAgentRuntimeIdentity: true, ...(signal ? { signal } : {}) }, + ); + return jsonResult(result); + } + if (action !== "request") { + throw new ToolInputError(`Unknown secrets action: ${action}`); + } + const request = normalizeSecretsRequestParams(input); + const delivery = beginAskUserPromptDelivery({ + toolCallId, + sessionKey: params.sessionKey, + runId: params.runId, + agentId: params.agentId, + questions: request.questions, + timeoutSeconds: request.timeoutSeconds, + }); + const timeoutMs = request.timeoutSeconds * 1_000; + let registered = false; + const cancelPendingQuestion = createGatewayQuestionCanceller({ + gatewayCall, + questionId: delivery.questionId, + }); + const cancelOnAbort = () => { + delivery.release(); + void cancelPendingQuestion("run-abort"); + }; + try { + signal?.throwIfAborted(); + const registration = asNullableRecord( + await gatewayCall( + "question.request", + {}, + { + id: delivery.questionId, + questions: request.questions, + ...(params.agentId ? { agentId: params.agentId } : {}), + ...(params.sessionKey ? { sessionKey: params.sessionKey } : {}), + ...(params.runId ? { runId: params.runId } : {}), + timeoutMs, + }, + // Store-bound requests are gated on an admin client server-side; the + // default least-privilege scope for question.request is not enough. + { scopes: [ADMIN_SCOPE], ...(signal ? { signal } : {}) }, + ), + ); + registered = true; + if (registration?.id !== delivery.questionId) { + throw new Error("question.request returned an unexpected question id"); + } + const record = await gatewayCall( + "question.get", + {}, + { id: delivery.questionId }, + signal ? { signal } : undefined, + ).catch(() => undefined); + const questionRecord = asNullableRecord(asNullableRecord(record)?.question); + const questions = questionRecord?.questions; + const replacedExisting = + Array.isArray(questions) && + asNullableRecord(questions[0])?.secretStoreExisting !== undefined; + signal?.addEventListener("abort", cancelOnAbort, { once: true }); + if (signal?.aborted) { + cancelOnAbort(); + signal.throwIfAborted(); + } + const answerPromise = awaitGatewayQuestionAnswer({ + gatewayCall, + questionId: delivery.questionId, + timeoutMs, + ...(signal ? { signal } : {}), + }); + delivery.markReady(); + if (delivery.hasSubscriber) { + const first = await Promise.race([ + delivery.waitForDelivery(signal).then((result) => ({ + kind: "delivery" as const, + result, + })), + answerPromise.then((result) => ({ kind: "answer" as const, result })), + ]); + if (first.kind === "delivery" && first.result.error !== undefined) { + await cancelPendingQuestion("prompt-delivery-failed"); + throw new Error("credential-request prompt delivery failed", { + cause: first.result.error, + }); + } + } + const result = await answerPromise; + signal?.throwIfAborted(); + if (result.status === "answered") { + if (result.answers.answers.secret_value?.[0] !== "stored") { + throw new Error("credential request returned an unexpected answer marker"); + } + return storedSecretResult(request, replacedExisting); + } + if (result.status === "pending") { + // The human may have submitted between the wait timeout and this + // cancel; the Gateway then rejects the cancel and hands back the + // answer, which means the credential is already stored. + const answered = await cancelPendingQuestion("wait-timeout"); + if (answered) { + return storedSecretResult(request, replacedExisting); + } + } + if ( + result.status === "pending" || + result.status === "expired" || + result.status === "cancelled" + ) { + return noSecretAnswerResult(result.status); + } + throw new Error("question.waitAnswer returned an invalid status"); + } catch (error) { + if (registered || signal?.aborted) { + await cancelPendingQuestion(signal?.aborted ? "run-abort" : "tool-error"); + } + throw error; + } finally { + signal?.removeEventListener("abort", cancelOnAbort); + delivery.release(); + } + }, + }; +} diff --git a/src/cli/capability-cli/image.ts b/src/cli/capability-cli/image.ts index 4dd3a8900694..64dd9850c38c 100644 --- a/src/cli/capability-cli/image.ts +++ b/src/cli/capability-cli/image.ts @@ -26,7 +26,6 @@ import { } from "../../media-understanding/runtime.js"; import { getImageMetadata } from "../../media/media-services.js"; import { defaultRuntime } from "../../runtime.js"; -import { formatHumanList } from "../../shared/human-list.js"; import { runCommandWithRuntime } from "../cli-utils.js"; import { getModelsCommandSecretTargetIds } from "../command-secret-targets.js"; import { readInputFiles, writeOutputAsset } from "../media-output.js"; @@ -49,8 +48,6 @@ import { const IMAGE_OUTPUT_FORMATS = ["png", "jpeg", "webp"] as const; const IMAGE_BACKGROUNDS = ["transparent", "opaque", "auto"] as const; -const IMAGE_QUALITIES = ["low", "medium", "high", "auto"] as const; -const IMAGE_OPENAI_MODERATIONS = ["low", "auto"] as const; async function runImageGenerate(params: { capability: "image.generate" | "image.edit"; @@ -235,20 +232,60 @@ async function runImageDescribe(params: { } satisfies CapabilityEnvelope; } -function normalizeImageOption( +function normalizeImageOutputFormat( raw: string | undefined, - values: readonly T[], - label: string, -): T | undefined { +): ImageGenerationOutputFormat | undefined { const normalized = normalizeLowercaseStringOrEmpty(raw); if (!normalized) { return undefined; } - const match = values.find((value) => value === normalized); - if (match) { - return match; + if ((IMAGE_OUTPUT_FORMATS as readonly string[]).includes(normalized)) { + return normalized as ImageGenerationOutputFormat; } - throw new Error(`${label} must be one of ${formatHumanList(values)}`); + throw new Error("--output-format must be one of png, jpeg, or webp"); +} + +function normalizeImageBackground( + raw: string | undefined, + label = "--background", +): ImageGenerationBackground | undefined { + const normalized = normalizeLowercaseStringOrEmpty(raw); + if (!normalized) { + return undefined; + } + if ((IMAGE_BACKGROUNDS as readonly string[]).includes(normalized)) { + return normalized as ImageGenerationBackground; + } + throw new Error(`${label} must be one of transparent, opaque, or auto`); +} + +function normalizeImageQuality(raw: string | undefined): ImageGenerationQuality | undefined { + const normalized = normalizeLowercaseStringOrEmpty(raw); + if (!normalized) { + return undefined; + } + if ( + normalized === "low" || + normalized === "medium" || + normalized === "high" || + normalized === "auto" + ) { + return normalized; + } + throw new Error("--quality must be one of low, medium, high, or auto"); +} + +function normalizeOpenAIModeration( + raw: string | undefined, +): ImageGenerationOpenAIModeration | undefined { + const normalized = normalizeLowercaseStringOrEmpty(raw); + if (!normalized) { + return undefined; + } + if (normalized === "low" || normalized === "auto") { + return normalized; + } + throw new Error("--openai-moderation must be one of low or auto"); } function resolveImageDescribeInput(filePath: string): string { @@ -277,6 +314,11 @@ function addImageGenerationOptions(command: Command): Command { .option("--json", "Output JSON", false); } +function readStringOption(opts: Record, key: string): string | undefined { + const value = opts[key]; + return typeof value === "string" ? value : undefined; +} + function resolveImageGenerationOptions(opts: Record, command: Command) { return { agent: resolveCapabilityAgentOption(command, opts.agent), @@ -285,27 +327,14 @@ function resolveImageGenerationOptions(opts: Record, command: C size: opts.size as string | undefined, aspectRatio: opts.aspectRatio as string | undefined, resolution: opts.resolution as "1K" | "2K" | "4K" | undefined, - outputFormat: normalizeImageOption( - opts.outputFormat as string | undefined, - IMAGE_OUTPUT_FORMATS, - "--output-format", - ), - background: normalizeImageOption( - opts.background as string | undefined, - IMAGE_BACKGROUNDS, - "--background", - ), - openaiBackground: normalizeImageOption( + outputFormat: normalizeImageOutputFormat(readStringOption(opts, "outputFormat")), + background: normalizeImageBackground(readStringOption(opts, "background")), + openaiBackground: normalizeImageBackground( opts.openaiBackground as string | undefined, - IMAGE_BACKGROUNDS, "--openai-background", ), - openaiModeration: normalizeImageOption( - opts.openaiModeration as string | undefined, - IMAGE_OPENAI_MODERATIONS, - "--openai-moderation", - ), - quality: normalizeImageOption(opts.quality as string | undefined, IMAGE_QUALITIES, "--quality"), + openaiModeration: normalizeOpenAIModeration(readStringOption(opts, "openaiModeration")), + quality: normalizeImageQuality(readStringOption(opts, "quality")), timeoutMs: parseOptionalTimeoutMs(opts.timeoutMs as string | number | undefined), output: opts.output as string | undefined, }; diff --git a/src/gateway/server-aux-handlers.ts b/src/gateway/server-aux-handlers.ts index 9877802cc4a1..851ae2bde95b 100644 --- a/src/gateway/server-aux-handlers.ts +++ b/src/gateway/server-aux-handlers.ts @@ -148,12 +148,26 @@ export function createGatewayAuxHandlers( ), { cacheRejections: true }, ); + const reloadSecrets = createGatewaySecretsReloader(params); + const loadSecretsModule = createLazyPromise(() => import("./server-methods/secrets.js"), { + cacheRejections: true, + }); + const loadSecretStoreWriteService = createLazyPromise( + async () => { + const { createSecretStoreWriteService } = await loadSecretsModule(); + return createSecretStoreWriteService({ reloadSecrets, log: params.log }); + }, + { cacheRejections: true }, + ); const questionManager = new QuestionManager(); const loadQuestionHandlers = createLazyPromise( - () => - import("./server-methods/question.js").then(({ createQuestionHandlers }) => - createQuestionHandlers(questionManager), - ), + async () => { + const [{ createQuestionHandlers }, storeWriteService] = await Promise.all([ + import("./server-methods/question.js"), + loadSecretStoreWriteService(), + ]); + return createQuestionHandlers(questionManager, storeWriteService); + }, { cacheRejections: true }, ); const pluginApprovalManager = createApprovalManager( @@ -302,41 +316,43 @@ export function createGatewayAuxHandlers( { cacheRejections: true }, ); const loadSecretsHandlers = createLazyPromise( - () => - import("./server-methods/secrets.js").then(({ createSecretsHandlers }) => - createSecretsHandlers({ - reloadSecrets: createGatewaySecretsReloader(params), - log: params.log, - resolveSecrets: async ({ - allowedPaths, - commandName, - forcedActivePaths, - optionalActivePaths, - providerOverrides, - targetIds, - }) => { - const { assignments, diagnostics, inactiveRefPaths } = - await resolveCommandSecretsFromActiveRuntimeSnapshot({ - commandName, - targetIds: new Set(targetIds), - ...(allowedPaths ? { allowedPaths: new Set(allowedPaths) } : {}), - ...(forcedActivePaths ? { forcedActivePaths: new Set(forcedActivePaths) } : {}), - ...(optionalActivePaths - ? { optionalActivePaths: new Set(optionalActivePaths) } - : {}), - ...(providerOverrides ? { providerOverrides } : {}), - }); - if (assignments.length === 0) { - return { - assignments: [] as CommandSecretAssignment[], - diagnostics, - inactiveRefPaths, - }; - } - return { assignments, diagnostics, inactiveRefPaths }; - }, - }), - ), + async () => { + const [{ createSecretsHandlers }, storeWriteService] = await Promise.all([ + loadSecretsModule(), + loadSecretStoreWriteService(), + ]); + return createSecretsHandlers({ + reloadSecrets, + storeWriteService, + log: params.log, + resolveSecrets: async ({ + allowedPaths, + commandName, + forcedActivePaths, + optionalActivePaths, + providerOverrides, + targetIds, + }) => { + const { assignments, diagnostics, inactiveRefPaths } = + await resolveCommandSecretsFromActiveRuntimeSnapshot({ + commandName, + targetIds: new Set(targetIds), + ...(allowedPaths ? { allowedPaths: new Set(allowedPaths) } : {}), + ...(forcedActivePaths ? { forcedActivePaths: new Set(forcedActivePaths) } : {}), + ...(optionalActivePaths ? { optionalActivePaths: new Set(optionalActivePaths) } : {}), + ...(providerOverrides ? { providerOverrides } : {}), + }); + if (assignments.length === 0) { + return { + assignments: [] as CommandSecretAssignment[], + diagnostics, + inactiveRefPaths, + }; + } + return { assignments, diagnostics, inactiveRefPaths }; + }, + }); + }, { cacheRejections: true }, ); diff --git a/src/gateway/server-methods/question.test.ts b/src/gateway/server-methods/question.test.ts index e894a05885b1..9ed061c5b3ad 100644 --- a/src/gateway/server-methods/question.test.ts +++ b/src/gateway/server-methods/question.test.ts @@ -2,6 +2,10 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { upsertSessionEntryCore } from "../../config/sessions/session-accessor.js"; import { addSessionMember } from "../../config/sessions/session-sharing-store.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; +import { clearAgentRunContext, registerAgentRunContext } from "../../infra/agent-run-registry.js"; +import { isSecretValueRegisteredForRedaction } from "../../logging/secret-redaction-registry.js"; +import * as secretsRuntimeState from "../../secrets/runtime-state.js"; +import { listSecretStoreEntries, writeSecretStoreEntry } from "../../secrets/store/secret-store.js"; import { ensureProfileForEmail, setUserProfileRole } from "../../state/user-profiles.js"; import { withOpenClawTestState } from "../../test-utils/openclaw-test-state.js"; import { QuestionManager } from "../question-manager.js"; @@ -9,22 +13,34 @@ import { createGatewayBroadcaster } from "../server-broadcast.js"; import type { GatewayWsClient } from "../server/ws-types.js"; import { canReceiveSessionEvent } from "../session-sharing.js"; import { createQuestionHandlers } from "./question.js"; +import { createSecretStoreWriteService } from "./secrets.js"; import type { GatewayClient, GatewayRequestHandlerOptions, RespondFn } from "./types.js"; let manager: QuestionManager; let broadcast: ReturnType; let handlers: ReturnType; +type SecretStoreReload = Parameters[0]["reloadSecrets"]; +let reloadSecrets: ReturnType>; beforeEach(() => { + // Store-bound resolution revalidates the requesting run at the write, so the + // fixtures must present the live run the questions are bound to. + registerAgentRunContext(requestParams.runId, { + sessionKey: requestParams.sessionKey, + agentId: requestParams.agentId, + }); vi.useFakeTimers(); vi.setSystemTime(1_000); manager = new QuestionManager(); broadcast = vi.fn(); - handlers = createQuestionHandlers(manager); + reloadSecrets = vi.fn().mockResolvedValue({ warningCount: 0 }); + handlers = createQuestionHandlers(manager, createSecretStoreWriteService({ reloadSecrets })); }); afterEach(() => { + clearAgentRunContext(requestParams.runId); manager.reset(); + vi.restoreAllMocks(); vi.useRealTimers(); }); @@ -71,6 +87,30 @@ const requestParams = { timeoutMs: 100, }; +const secretRequestQuestion = { + questionId: "secret_value", + header: "API key", + question: "Provide SERVICE_API_KEY", + options: [], + isSecret: true, + secretStore: { + name: "SERVICE_API_KEY", + kind: "secret" as const, + allowedHosts: ["api.example.test"], + }, +}; + +const secretRequestParams = { + ...requestParams, + questions: [secretRequestQuestion], +}; + +// Store-bound questions may only be minted by admin-scoped clients; every +// store-bound request below presents one so validation errors stay specific. +const adminRequestClient = { + connect: { scopes: ["operator.admin"] }, +} as GatewayClient; + describe("question gateway methods", () => { it("conceals foreign session questions for role-none readers while preserving global prompts", async () => { await withOpenClawTestState({ scenario: "minimal" }, async () => { @@ -389,6 +429,412 @@ describe("question gateway methods", () => { ); }); + it.each([ + { + behavior: "bindings without the secret-input marker", + questions: [{ ...secretRequestParams.questions[0], isSecret: false }], + }, + { + behavior: "secret requests mixed with another question", + questions: [secretRequestParams.questions[0], requestParams.questions[0]], + }, + { + behavior: "secret requests with answer options", + questions: [ + { + ...secretRequestParams.questions[0], + options: [{ label: "First" }, { label: "Second" }], + }, + ], + }, + { + behavior: "secret requests allowing multiple selections", + questions: [{ ...secretRequestParams.questions[0], multiSelect: true }], + }, + { + behavior: "invalid secret store entry names", + questions: [ + { + ...secretRequestParams.questions[0], + secretStore: { ...secretRequestQuestion.secretStore, name: "lowercase" }, + }, + ], + }, + { + behavior: "invalid secret store entry kinds", + questions: [ + { + ...secretRequestParams.questions[0], + secretStore: { ...secretRequestQuestion.secretStore, kind: "password" }, + }, + ], + }, + { + behavior: "more than 128 proposed allowed hosts", + questions: [ + { + ...secretRequestParams.questions[0], + secretStore: { + ...secretRequestQuestion.secretStore, + allowedHosts: Array.from({ length: 129 }, (_, index) => `${index}.example.test`), + }, + }, + ], + }, + { + behavior: "allowed hosts proposed for environment entries", + questions: [ + { + ...secretRequestParams.questions[0], + secretStore: { ...secretRequestQuestion.secretStore, kind: "env" }, + }, + ], + }, + ])("rejects $behavior before opening a pending secret question", async ({ questions }) => { + const response = await call( + "question.request", + { ...requestParams, questions }, + { client: adminRequestClient }, + ); + + expect(response).toMatchObject([false, undefined, { code: "INVALID_REQUEST" }]); + expect(manager.list()).toEqual([]); + }); + + it.each([ + { behavior: "a connect-less client", client: null }, + { + behavior: "a questions-scoped client", + client: { + connect: { scopes: ["operator.questions"] }, + } as GatewayClient, + }, + ])( + "refuses to mint store-bound questions for $behavior so questions scope cannot reach store writes", + async ({ client }) => { + const response = await call( + "question.request", + secretRequestParams, + client ? { client } : undefined, + ); + + expect(response).toMatchObject([ + false, + undefined, + { code: "INVALID_REQUEST", message: expect.stringContaining("operator.admin") }, + ]); + expect(manager.list()).toEqual([]); + }, + ); + + it("refuses to write a credential once its requesting run is gone", async () => { + await withOpenClawTestState({ scenario: "minimal" }, async () => { + const requested = await call("question.request", secretRequestParams, { + client: adminRequestClient, + }); + const id = (requested[1] as { id: string }).id; + // The requester dies between the prompt and the human's submission. + clearAgentRunContext(requestParams.runId); + + const resolved = await call("question.resolve", { + id, + answers: { answers: { secret_value: ["test-secret-value-stale-runner-123"] } }, + }); + + expect(resolved).toMatchObject([ + false, + undefined, + { code: "INVALID_REQUEST", details: { reason: "QUESTION_REQUESTER_INACTIVE" } }, + ]); + expect(listSecretStoreEntries({ scope: { kind: "team" } })).toEqual([]); + expect(manager.get(id)?.status).toBe("pending"); + }); + }); + + it("refuses to mint a store-bound question that names no requesting run", async () => { + const { runId: _runId, ...withoutRun } = secretRequestParams; + + expect( + await call("question.request", withoutRun, { client: adminRequestClient }), + ).toMatchObject([ + false, + undefined, + { code: "INVALID_REQUEST", message: expect.stringContaining("runId") }, + ]); + expect(manager.list()).toEqual([]); + }); + + it("annotates a store-bound question with replacement metadata without exposing the old value", async () => { + await withOpenClawTestState({ scenario: "minimal" }, async () => { + const oldValue = "test-secret-value-existing-123"; + writeSecretStoreEntry({ + scope: { kind: "team" }, + name: "SERVICE_API_KEY", + value: oldValue, + kind: "secret", + updatedBy: "Previous Operator", + }); + + const response = await call("question.request", secretRequestParams, { + client: adminRequestClient, + }); + const id = (response[1] as { id: string }).id; + const record = manager.get(id); + + expect(record?.questions[0]).toMatchObject({ + secretStore: secretRequestQuestion.secretStore, + secretStoreExisting: { updatedAtMs: 1_000, updatedBy: "Previous Operator" }, + }); + expect(JSON.stringify(record)).not.toContain(oldValue); + }); + }); + + it("diverts operator-entered credentials into the store and exposes only a stored marker", async () => { + await withOpenClawTestState({ scenario: "minimal" }, async () => { + const response = await call("question.request", secretRequestParams, { + client: adminRequestClient, + }); + const id = (response[1] as { id: string }).id; + const value = "test-secret-value-gateway-diversion-123"; + const client = { + connect: { client: { displayName: "Trusted Operator" } }, + } as GatewayClient; + + const resolved = await call( + "question.resolve", + { id, answers: { answers: { secret_value: [value] } }, resolvedBy: "control-ui" }, + { client }, + ); + const safeAnswers = { answers: { secret_value: ["stored"] } }; + + expect(resolved).toEqual([true, { status: "answered", answers: safeAnswers }, undefined]); + expect(listSecretStoreEntries({ scope: { kind: "team" } })).toMatchObject([ + { + name: "SERVICE_API_KEY", + kind: "secret", + allowedHosts: ["api.example.test"], + updatedBy: "Trusted Operator", + }, + ]); + expect(manager.get(id)).toMatchObject({ status: "answered", answers: safeAnswers }); + expect(await call("question.waitAnswer", { id })).toEqual([ + true, + { status: "answered", answers: safeAnswers }, + undefined, + ]); + expect(broadcast).toHaveBeenCalledWith("question.resolved", { + id, + status: "answered", + answers: safeAnswers, + }); + expect(JSON.stringify([resolved, manager.get(id), broadcast.mock.calls])).not.toContain( + value, + ); + expect(isSecretValueRegisteredForRedaction(value)).toBe(true); + }); + }); + + it("uses operator-edited hosts and keeps invalid store submissions pending for retry", async () => { + await withOpenClawTestState({ scenario: "minimal" }, async () => { + const requested = await call("question.request", secretRequestParams, { + client: adminRequestClient, + }); + const id = (requested[1] as { id: string }).id; + const value = "test-secret-value-retry-123"; + const answers = { answers: { secret_value: [value] } }; + + const invalid = await call("question.resolve", { + id, + answers, + secretStoreAllowedHosts: ["*.example.test"], + }); + expect(invalid).toMatchObject([ + false, + undefined, + { code: "INVALID_REQUEST", message: expect.stringContaining("wildcard") }, + ]); + expect(manager.get(id)?.status).toBe("pending"); + expect(isSecretValueRegisteredForRedaction(value)).toBe(true); + + const retried = await call("question.resolve", { + id, + answers, + secretStoreAllowedHosts: ["replacement.example.test"], + }); + expect(retried[0]).toBe(true); + expect(listSecretStoreEntries({ scope: { kind: "team" } })[0]).toMatchObject({ + allowedHosts: ["replacement.example.test"], + }); + }); + }); + + it.each([ + { behavior: "no submitted value", answers: { secret_value: [] } }, + { + behavior: "multiple submitted values", + answers: { secret_value: ["test-secret-value-first", "test-secret-value-second"] }, + }, + { + behavior: "an unrelated submitted answer", + answers: { secret_value: ["test-secret-value-only"], destination: ["Home"] }, + }, + ])("keeps a secret question pending when there is $behavior", async ({ answers }) => { + await withOpenClawTestState({ scenario: "minimal" }, async () => { + const requested = await call("question.request", secretRequestParams, { + client: adminRequestClient, + }); + const id = (requested[1] as { id: string }).id; + + expect(await call("question.resolve", { id, answers: { answers } })).toMatchObject([ + false, + undefined, + { code: "INVALID_REQUEST" }, + ]); + expect(manager.get(id)?.status).toBe("pending"); + expect(listSecretStoreEntries({ scope: { kind: "team" } })).toEqual([]); + }); + }); + + it("rejects host overrides on env entries and ordinary questions without settling them", async () => { + await withOpenClawTestState({ scenario: "minimal" }, async () => { + const envQuestion = { + ...secretRequestParams.questions[0], + secretStore: { name: "SERVICE_URL", kind: "env" as const }, + }; + const envResponse = await call( + "question.request", + { ...secretRequestParams, questions: [envQuestion] }, + { client: adminRequestClient }, + ); + const envId = (envResponse[1] as { id: string }).id; + + expect( + await call("question.resolve", { + id: envId, + answers: { answers: { secret_value: ["https://example.test"] } }, + secretStoreAllowedHosts: ["example.test"], + }), + ).toMatchObject([false, undefined, { code: "INVALID_REQUEST" }]); + expect(manager.get(envId)?.status).toBe("pending"); + + const ordinaryResponse = await call("question.request", requestParams); + const ordinaryId = (ordinaryResponse[1] as { id: string }).id; + expect( + await call("question.resolve", { + id: ordinaryId, + answers: { answers: { destination: ["Home"] } }, + secretStoreAllowedHosts: ["example.test"], + }), + ).toMatchObject([false, undefined, { code: "INVALID_REQUEST" }]); + expect(manager.get(ordinaryId)?.status).toBe("pending"); + }); + }); + + it("stores environment entries without host policy and preserves secret-question cancellation", async () => { + await withOpenClawTestState({ scenario: "minimal" }, async () => { + const envQuestion = { + ...secretRequestParams.questions[0], + secretStore: { name: "SERVICE_URL", kind: "env" as const }, + }; + const envResponse = await call( + "question.request", + { ...secretRequestParams, questions: [envQuestion] }, + { client: adminRequestClient }, + ); + const envId = (envResponse[1] as { id: string }).id; + expect( + ( + await call("question.resolve", { + id: envId, + answers: { answers: { secret_value: ["https://example.test"] } }, + }) + )[0], + ).toBe(true); + expect(listSecretStoreEntries({ scope: { kind: "team" } })[0]).toMatchObject({ + name: "SERVICE_URL", + kind: "env", + valuePreview: "https://example.test", + }); + + const cancelledResponse = await call("question.request", secretRequestParams, { + client: adminRequestClient, + }); + const cancelledId = (cancelledResponse[1] as { id: string }).id; + expect(await call("question.resolve", { id: cancelledId, cancel: true })).toEqual([ + true, + { status: "cancelled" }, + undefined, + ]); + expect(manager.get(cancelledId)?.status).toBe("cancelled"); + }); + }); + + it("cold-refreshes configured SecretRefs after a store-bound question is answered", async () => { + await withOpenClawTestState({ scenario: "minimal" }, async () => { + vi.spyOn(secretsRuntimeState, "getActiveSecretsRuntimeSnapshotState").mockReturnValue({ + sourceConfig: { + models: { + providers: { + test: { + baseUrl: "https://provider.example.test", + models: [], + apiKey: { source: "store", provider: "default", id: "SERVICE_API_KEY" }, + }, + }, + }, + }, + config: {}, + authStores: [], + authStoreCredentialsRevision: 0, + warnings: [], + webTools: { + search: { providerSource: "none", diagnostics: [] }, + fetch: { providerSource: "none", diagnostics: [] }, + diagnostics: [], + }, + }); + const requested = await call("question.request", secretRequestParams, { + client: adminRequestClient, + }); + const id = (requested[1] as { id: string }).id; + + expect( + ( + await call("question.resolve", { + id, + answers: { answers: { secret_value: ["test-secret-value-cold-refresh-123"] } }, + }) + )[0], + ).toBe(true); + expect(reloadSecrets).toHaveBeenCalledWith({ + forceColdRefKeys: new Set(["store:default:SERVICE_API_KEY"]), + joinInFlight: false, + }); + }); + }); + + it("keeps store-bound questions pending when the write service is unavailable", async () => { + await withOpenClawTestState({ scenario: "minimal" }, async () => { + const service = createSecretStoreWriteService({ reloadSecrets }); + handlers = createQuestionHandlers(manager, { + ...service, + write: vi.fn().mockRejectedValue(new Error("database unavailable")), + }); + const requested = await call("question.request", secretRequestParams, { + client: adminRequestClient, + }); + const id = (requested[1] as { id: string }).id; + + expect( + await call("question.resolve", { + id, + answers: { answers: { secret_value: ["test-secret-value-unavailable-123"] } }, + }), + ).toMatchObject([false, undefined, { code: "UNAVAILABLE" }]); + expect(manager.get(id)?.status).toBe("pending"); + }); + }); + it("returns INVALID_REQUEST for answers that violate the stored question", async () => { const requested = await call("question.request", { ...requestParams, diff --git a/src/gateway/server-methods/question.ts b/src/gateway/server-methods/question.ts index 5030bc287f3b..c77b96b7c714 100644 --- a/src/gateway/server-methods/question.ts +++ b/src/gateway/server-methods/question.ts @@ -14,10 +14,18 @@ import { validateQuestionWaitAnswerParams, } from "../../../packages/gateway-protocol/src/index.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; +import { ENV_SECRET_REF_ID_RE } from "../../config/types.secrets.js"; +import { getAgentRunContext } from "../../infra/agent-run-registry.js"; import { handleQuestionChannelRequested, handleQuestionChannelResolved, } from "../../infra/question-channel-runtime.js"; +import { registerSecretValueForRedaction } from "../../logging/secret-redaction-registry.js"; +import { + listSecretStoreEntries, + SECRET_STORE_ALLOWED_HOSTS_MAX, + SecretStoreValidationError, +} from "../../secrets/store/secret-store.js"; import { hasOperatorBoundary } from "../operator-role-policy.js"; import { QuestionManager, @@ -33,6 +41,7 @@ import { resolveSessionSharingTarget, } from "../session-sharing.js"; import { resolveStoredSessionKeyForAgentStore } from "../session-store-key.js"; +import type { SecretStoreWriteService } from "./secrets.js"; import type { GatewayClient, GatewayRequestHandlers, RespondFn } from "./types.js"; const DEFAULT_QUESTION_TIMEOUT_MS = 15 * 60 * 1_000; @@ -117,11 +126,54 @@ function normalizeQuestions(params: QuestionRequestParams): Question[] { `question '${question.questionId}' must have either no options or 2 to 4 options`, ); } - if (question.isSecret) { + const binding = question.secretStore; + if (question.isSecret && !binding) { throw new QuestionRequestValidationError( `question '${question.questionId}': secret questions are not supported yet`, ); } + if (binding) { + if (!question.isSecret) { + throw new QuestionRequestValidationError( + `question '${question.questionId}': secret store binding requires a secret question`, + ); + } + if (params.questions.length !== 1 || question.options.length !== 0 || question.multiSelect) { + throw new QuestionRequestValidationError( + `question '${question.questionId}': secret store requests require one free-text, single-select question`, + ); + } + if (!ENV_SECRET_REF_ID_RE.test(binding.name)) { + throw new QuestionRequestValidationError( + `question '${question.questionId}': invalid secret store entry name`, + ); + } + if (binding.kind !== "secret" && binding.kind !== "env") { + throw new QuestionRequestValidationError( + `question '${question.questionId}': invalid secret store entry kind`, + ); + } + if ((binding.allowedHosts?.length ?? 0) > SECRET_STORE_ALLOWED_HOSTS_MAX) { + throw new QuestionRequestValidationError( + `question '${question.questionId}': secret store allowed hosts exceed the limit`, + ); + } + if (binding.kind === "env" && binding.allowedHosts !== undefined) { + throw new QuestionRequestValidationError("Allowed hosts apply only to secret entries."); + } + const existing = listSecretStoreEntries({ scope: { kind: "team" } }).find( + (entry) => entry.name === binding.name, + ); + if (existing) { + return { + ...question, + secretStoreExisting: { + updatedAtMs: existing.updatedAtMs, + ...(existing.updatedBy ? { updatedBy: existing.updatedBy } : {}), + }, + }; + } + } const optionLabels = new Set(); for (const option of question.options) { const normalizedLabel = option.label.trim().toLowerCase(); @@ -137,7 +189,10 @@ function normalizeQuestions(params: QuestionRequestParams): Question[] { } /** Creates the lazily loaded question RPC surface for one Gateway lifetime. */ -export function createQuestionHandlers(manager: QuestionManager): GatewayRequestHandlers { +export function createQuestionHandlers( + manager: QuestionManager, + storeWriteService: SecretStoreWriteService, +): GatewayRequestHandlers { return { "question.request": ({ params, respond, context, client }) => { if (!validateQuestionRequestParams(params)) { @@ -145,6 +200,31 @@ export function createQuestionHandlers(manager: QuestionManager): GatewayRequest return; } const request = params as QuestionRequestParams; + // Store-bound questions end in a secret-store write on resolve. Without + // this gate any operator.questions client could mint and self-answer one, + // bypassing the operator.admin requirement on secrets.store.set. + if (request.questions.some((question) => question.secretStore) && !request.runId) { + respond( + false, + undefined, + errorShape( + ErrorCodes.INVALID_REQUEST, + "secret store questions must carry the requesting runId", + ), + ); + return; + } + if (request.questions.some((question) => question.secretStore) && !isGatewayAdmin(client)) { + respond( + false, + undefined, + errorShape( + ErrorCodes.INVALID_REQUEST, + "secret store questions require an operator.admin client", + ), + ); + return; + } try { const requestedSession = request.sessionKey ? resolveRequestedSessionAgentId( @@ -216,6 +296,14 @@ export function createQuestionHandlers(manager: QuestionManager): GatewayRequest return; } if (!managerError(error, respond)) { + if (request.questions.some((question) => question.secretStore)) { + respond( + false, + undefined, + errorShape(ErrorCodes.UNAVAILABLE, "Secret store entry metadata is unavailable."), + ); + return; + } throw error; } } @@ -261,7 +349,7 @@ export function createQuestionHandlers(manager: QuestionManager): GatewayRequest } } }, - "question.resolve": ({ params, respond, client, context }) => { + "question.resolve": async ({ params, respond, client, context }) => { if (!validateQuestionResolveParams(params)) { validationError("question.resolve", validateQuestionResolveParams.errors, respond); return; @@ -281,10 +369,113 @@ export function createQuestionHandlers(manager: QuestionManager): GatewayRequest return; } } - const result = - "cancel" in request - ? manager.cancel(request.id, request.resolvedBy) - : manager.resolve(request.id, request.answers, request.resolvedBy); + if ("cancel" in request) { + respond(true, manager.cancel(request.id, request.resolvedBy), undefined); + return; + } + const secretQuestion = question?.questions[0]; + const binding = secretQuestion?.secretStore; + if (!binding || !question) { + if (request.secretStoreAllowedHosts !== undefined) { + respond( + false, + undefined, + errorShape( + ErrorCodes.INVALID_REQUEST, + "Secret store allowed hosts require a store-bound question.", + ), + ); + return; + } + respond( + true, + manager.resolve(request.id, request.answers, request.resolvedBy), + undefined, + ); + return; + } + if (question.status !== "pending") { + throw new QuestionManagerError( + QuestionManagerErrorCodes.ALREADY_TERMINAL, + `question '${request.id}' is already ${question.status}`, + ); + } + const submittedAnswers = request.answers.answers; + const values = Object.hasOwn(submittedAnswers, secretQuestion.questionId) + ? submittedAnswers[secretQuestion.questionId] + : undefined; + const value = values?.[0]; + if ( + Object.keys(submittedAnswers).length !== 1 || + values?.length !== 1 || + value === undefined + ) { + respond( + false, + undefined, + errorShape( + ErrorCodes.INVALID_REQUEST, + `question '${secretQuestion.questionId}' requires exactly one secret value`, + ), + ); + return; + } + registerSecretValueForRedaction(value); + const allowedHosts = request.secretStoreAllowedHosts ?? binding.allowedHosts; + if (binding.kind === "env" && allowedHosts !== undefined) { + respond( + false, + undefined, + errorShape(ErrorCodes.INVALID_REQUEST, "Allowed hosts apply only to secret entries."), + ); + return; + } + // Closure-bound authority: a store write is delegated by one live agent + // run, so revalidate that exact run immediately before the sink. The + // recorded runId is provenance; a terminated or replaced requester must + // not reach secret-store I/O. No await may separate this from the write. + if (!question.runId || !getAgentRunContext(question.runId)) { + respond( + false, + undefined, + errorShape( + ErrorCodes.INVALID_REQUEST, + "the agent run that requested this credential is no longer active", + { details: { reason: "QUESTION_REQUESTER_INACTIVE" } }, + ), + ); + return; + } + try { + await storeWriteService.write({ + name: binding.name, + value, + kind: binding.kind, + ...(allowedHosts !== undefined ? { allowedHosts } : {}), + updatedBy: storeWriteService.resolveUpdatedBy(client), + }); + } catch (error) { + respond( + false, + undefined, + errorShape( + error instanceof SecretStoreValidationError + ? ErrorCodes.INVALID_REQUEST + : ErrorCodes.UNAVAILABLE, + error instanceof SecretStoreValidationError + ? error.message + : "Secret store entry could not be saved.", + ), + ); + return; + } + // Only the synthetic marker may enter manager state, event fanout, + // waiting agent responses, and channel-rendered question outcomes. + const result = manager.resolve( + request.id, + { answers: { [secretQuestion.questionId]: ["stored"] } }, + request.resolvedBy, + ); respond(true, result, undefined); } catch (error) { if (!managerError(error, respond)) { diff --git a/src/gateway/server-methods/secrets.test.ts b/src/gateway/server-methods/secrets.test.ts index ea791a4cbb76..55008f7cc096 100644 --- a/src/gateway/server-methods/secrets.test.ts +++ b/src/gateway/server-methods/secrets.test.ts @@ -45,11 +45,12 @@ vi.mock("../../secrets/target-registry.js", () => ({ isKnownSecretTargetId: () => false, })); +import { isSecretValueRegisteredForRedaction } from "../../logging/secret-redaction-registry.js"; import { TALK_TEST_PROVIDER_API_KEY_PATH, TALK_TEST_PROVIDER_API_KEY_PATH_SEGMENTS, } from "../../test-utils/talk-test-provider.js"; -import { createSecretsHandlers } from "./secrets.js"; +import { createSecretsHandlers, createSecretStoreWriteService } from "./secrets.js"; async function invokeSecretsReload(params: { handlers: ReturnType; @@ -211,6 +212,7 @@ describe("secrets handlers", () => { })); return createSecretsHandlers({ reloadSecrets, + storeWriteService: createSecretStoreWriteService({ reloadSecrets, log: overrides?.log }), resolveSecrets, log: overrides?.log, }); @@ -455,6 +457,25 @@ describe("secrets handlers", () => { }); }); + it("registers submitted store values for redaction before a failing write", async () => { + const value = "test-secret-value-redaction-before-write-123"; + storeMocks.writeEntry.mockImplementationOnce(() => { + expect(isSecretValueRegisteredForRedaction(value)).toBe(true); + throw new Error("database unavailable"); + }); + const respond = vi.fn(); + + await invokeStoreMethod({ + handlers: createHandlers(), + method: "secrets.store.set", + requestParams: { name: "SERVICE_API_KEY", value, kind: "secret" }, + respond, + }); + + expectRespondError(respond, { code: "UNAVAILABLE", message: "secrets.store.set failed" }); + expect(isSecretValueRegisteredForRedaction(value)).toBe(true); + }); + it("rejects invalid store params before writing", async () => { const respond = vi.fn(); await invokeStoreMethod({ diff --git a/src/gateway/server-methods/secrets.ts b/src/gateway/server-methods/secrets.ts index ad7844044315..e568006b3c6b 100644 --- a/src/gateway/server-methods/secrets.ts +++ b/src/gateway/server-methods/secrets.ts @@ -14,6 +14,7 @@ import { type SecretStoreEntry, } from "../../../packages/gateway-protocol/src/index.js"; import { formatErrorMessage as errorMessage } from "../../infra/errors.js"; +import { registerSecretValueForRedaction } from "../../logging/secret-redaction-registry.js"; import { collectSecretStoreRefKeysInConfig, getActiveSecretsRuntimeSnapshotState, @@ -60,6 +61,72 @@ function storeUpdatedBy(client: GatewayClient | null): string { ); } +type SecretStoreReload = (options?: { + forceColdRefKeys?: ReadonlySet; + joinInFlight?: boolean; +}) => Promise<{ warningCount: number }>; + +type SecretStoreLogger = { + warn?: (message: string) => void; + debug?: (message: string) => void; +}; + +class SecretStorePostWriteError extends Error { + constructor(cause: unknown) { + super(errorMessage(cause), { cause }); + this.name = "SecretStorePostWriteError"; + } +} + +/** Owns redaction-first store writes and the runtime refresh shared by Gateway RPCs. */ +export function createSecretStoreWriteService(params: { + reloadSecrets: SecretStoreReload; + log?: SecretStoreLogger; +}) { + const purgeRetention = () => { + try { + purgeExpiredSecretStoreEntries(); + } catch (error) { + params.log?.warn?.(`secrets.store retention purge failed: ${errorMessage(error)}`); + } + }; + const reloadReference = async ( + name: string, + ): Promise<{ reloaded: boolean; warningCount?: number }> => { + const snapshot = getActiveSecretsRuntimeSnapshotState(); + const refKeys = snapshot + ? collectSecretStoreRefKeysInConfig(snapshot.sourceConfig, name) + : new Set(); + if (refKeys.size === 0) { + return { reloaded: false }; + } + // Explicit replacement must cold-refresh affected owners instead of + // retaining an older credential from the active runtime snapshot. + const reload = await params.reloadSecrets({ forceColdRefKeys: refKeys, joinInFlight: false }); + return { reloaded: true, warningCount: reload.warningCount }; + }; + + return { + resolveUpdatedBy: storeUpdatedBy, + purgeRetention, + reloadReference, + async write(input: Omit[0], "scope" | "database">) { + // Registration precedes validation and SQLite so even write failures + // cannot disclose the submitted credential through downstream logging. + registerSecretValueForRedaction(input.value); + writeSecretStoreEntry({ scope: teamScope, ...input }); + purgeRetention(); + try { + return await reloadReference(input.name); + } catch (error) { + throw new SecretStorePostWriteError(error); + } + }, + }; +} + +export type SecretStoreWriteService = ReturnType; + function invalidSecretsResolveField( errors: ValidationError[] | null | undefined, ): @@ -99,10 +166,8 @@ function invalidSecretsResolveField( } export function createSecretsHandlers(params: { - reloadSecrets: (options?: { - forceColdRefKeys?: ReadonlySet; - joinInFlight?: boolean; - }) => Promise<{ warningCount: number }>; + reloadSecrets: SecretStoreReload; + storeWriteService: SecretStoreWriteService; resolveSecrets: (params: { commandName: string; targetIds: string[]; @@ -122,33 +187,8 @@ export function createSecretsHandlers(params: { diagnostics: string[]; inactiveRefPaths: string[]; }>; - log?: { - warn?: (message: string) => void; - }; + log?: SecretStoreLogger; }): GatewayRequestHandlers { - const purgeStoreRetention = () => { - try { - purgeExpiredSecretStoreEntries(); - } catch (error) { - params.log?.warn?.(`secrets.store retention purge failed: ${errorMessage(error)}`); - } - }; - const reloadStoreReference = async ( - name: string, - ): Promise<{ reloaded: boolean; warningCount?: number }> => { - const snapshot = getActiveSecretsRuntimeSnapshotState(); - const refKeys = snapshot - ? collectSecretStoreRefKeysInConfig(snapshot.sourceConfig, name) - : new Set(); - if (refKeys.size === 0) { - return { reloaded: false }; - } - // An explicit store mutation must not reuse an older credential if the - // replacement is missing or invalid; affected owners become cold instead. - const reload = await params.reloadSecrets({ forceColdRefKeys: refKeys, joinInFlight: false }); - return { reloaded: true, warningCount: reload.warningCount }; - }; - return { "secrets.reload": async ({ respond }) => { try { @@ -278,21 +318,16 @@ export function createSecretsHandlers(params: { ) { return; } - let stored = false; try { - writeSecretStoreEntry({ - scope: teamScope, + const reload = await params.storeWriteService.write({ name: requestParams.name, value: requestParams.value, kind: requestParams.kind, ...(requestParams.allowedHosts !== undefined ? { allowedHosts: requestParams.allowedHosts } : {}), - updatedBy: storeUpdatedBy(client), + updatedBy: params.storeWriteService.resolveUpdatedBy(client), }); - stored = true; - purgeStoreRetention(); - const reload = await reloadStoreReference(requestParams.name); const result = { ok: true as const, ...reload, @@ -312,14 +347,14 @@ export function createSecretsHandlers(params: { undefined, errorShape( ErrorCodes.UNAVAILABLE, - stored + error instanceof SecretStorePostWriteError ? "Secret store entry was saved, but post-write runtime refresh failed. Resolve provider errors and retry secrets.reload." : "secrets.store.set failed", ), ); } }, - "secrets.store.delete": async ({ params: requestParams, respond }) => { + "secrets.store.delete": async ({ params: requestParams, respond, client }) => { if ( !assertValidParams( requestParams, @@ -332,10 +367,14 @@ export function createSecretsHandlers(params: { } let deleted = false; try { + const agentId = client?.internal?.agentRuntimeIdentity?.agentId; + if (agentId) { + params.log?.debug?.(`secrets.store.delete requested by agent:${agentId}`); + } deleteSecretStoreEntry({ scope: teamScope, name: requestParams.name }); deleted = true; - purgeStoreRetention(); - const reload = await reloadStoreReference(requestParams.name); + params.storeWriteService.purgeRetention(); + const reload = await params.storeWriteService.reloadReference(requestParams.name); const result = { ok: true as const, ...reload, diff --git a/test/telegram-question-gateway.test.ts b/test/telegram-question-gateway.test.ts index 74a86c94f5bb..5d63efdb3f59 100644 --- a/test/telegram-question-gateway.test.ts +++ b/test/telegram-question-gateway.test.ts @@ -5,6 +5,7 @@ import { telegramOutbound } from "../extensions/telegram/api.js"; import { buildAgentHarnessQuestionPromptPayload } from "../src/agents/harness/user-input-bridge.js"; import { QuestionManager } from "../src/gateway/question-manager.js"; import { createQuestionHandlers } from "../src/gateway/server-methods/question.js"; +import { createSecretStoreWriteService } from "../src/gateway/server-methods/secrets.js"; import { callGatewayHandler } from "../src/gateway/server-methods/skills.test-helpers.js"; type QuestionGatewayCall = { method: string; params?: Record }; @@ -33,7 +34,10 @@ afterEach(() => { describe("Telegram question Gateway resolution", () => { it("resolves canonical option C when rendered option A repeats across blocks", async () => { const manager = new QuestionManager(); - const handlers = createQuestionHandlers(manager); + const handlers = createQuestionHandlers( + manager, + createSecretStoreWriteService({ reloadSecrets: async () => ({ warningCount: 0 }) }), + ); const gatewayCalls: string[] = []; const dispatch = async ({ method, params }: QuestionGatewayCall): Promise => { gatewayCalls.push(method); diff --git a/ui/src/app-route-paths.ts b/ui/src/app-route-paths.ts index 928aaa53683a..e504610a4889 100644 --- a/ui/src/app-route-paths.ts +++ b/ui/src/app-route-paths.ts @@ -9,6 +9,10 @@ export const INTERNAL_SESSION_PATH_PARAM = "__openclawSessionPath"; export const INTERNAL_MEMORY_PATH_PARAM = "__openclawMemoryPath"; export const INTERNAL_PLUGINS_PATH_PARAM = "__openclawPluginsPath"; export const INTERNAL_WORKBOARD_PATH_PARAM = "__openclawWorkboardPath"; +export const CONTROL_UI_DOCUMENT_ROUTE_PATHS = { + approval: "/approve", + question: "/ask", +} as const; export type MemoryRouteTab = "overview" | "memories" | "dreams" | "settings"; export type PluginsHubRouteTab = "installed" | "discover"; @@ -341,6 +345,9 @@ export function inferBasePathFromPathname(pathname: string): string { for (let index = 0; index < segments.length; index += 1) { const candidate = `/${segments.slice(index).join("/")}`; const routePath = routePaths.find((path) => normalizePath(path) === candidate); + const documentRoutePath = Object.values(CONTROL_UI_DOCUMENT_ROUTE_PATHS).find( + (path) => candidate === path || candidate.startsWith(`${path}/`), + ); const dynamicAgentRoute = agentRouteFromPath(candidate) !== null; const dynamicWorkboardRoute = workboardBoardIdFromPath(candidate) !== null; const dynamicMemoryRoute = memoryTabFromPath(candidate) !== null; @@ -349,6 +356,7 @@ export function inferBasePathFromPathname(pathname: string): string { const dynamicSessionRoute = sessionNamespace !== null; if ( !routePath && + !documentRoutePath && !dynamicAgentRoute && !dynamicWorkboardRoute && !dynamicMemoryRoute && @@ -358,22 +366,25 @@ export function inferBasePathFromPathname(pathname: string): string { continue; } const previousSegment = segments[index - 1]; - const dynamicRoutePath = dynamicAgentRoute - ? APP_ROUTE_DEFINITIONS.agents.path - : dynamicWorkboardRoute - ? APP_ROUTE_DEFINITIONS.workboard.path - : dynamicMemoryRoute - ? APP_ROUTE_DEFINITIONS.memory.path - : dynamicPluginsRoute - ? APP_ROUTE_DEFINITIONS.plugins.path - : sessionNamespace - ? APP_ROUTE_DEFINITIONS[sessionNamespace].path - : null; + const dynamicRoutePath = documentRoutePath + ? documentRoutePath + : dynamicAgentRoute + ? APP_ROUTE_DEFINITIONS.agents.path + : dynamicWorkboardRoute + ? APP_ROUTE_DEFINITIONS.workboard.path + : dynamicMemoryRoute + ? APP_ROUTE_DEFINITIONS.memory.path + : dynamicPluginsRoute + ? APP_ROUTE_DEFINITIONS.plugins.path + : sessionNamespace + ? APP_ROUTE_DEFINITIONS[sessionNamespace].path + : null; const firstRouteSegment = (routePath ?? dynamicRoutePath ?? "").split("/").find(Boolean); if ( index > 0 && previousSegment === firstRouteSegment && (candidate === routePath || + Boolean(documentRoutePath) || dynamicAgentRoute || dynamicWorkboardRoute || dynamicMemoryRoute || diff --git a/ui/src/app/app-root.ts b/ui/src/app/app-root.ts index 2a9d25a700b1..9482165d6702 100644 --- a/ui/src/app/app-root.ts +++ b/ui/src/app/app-root.ts @@ -28,6 +28,7 @@ import { isOptionalElementDefined, LazyCustomElementRequestController, type OptionalCustomElement, + QUESTION_PAGE_ELEMENT, TERMINAL_PANEL_ELEMENT, } from "./lazy-custom-element.ts"; import { resolveOnboardingMode } from "./onboarding-mode.ts"; @@ -140,6 +141,9 @@ export class OpenClawApp extends OpenClawLightDomElement { if (this.runtime.documentMode?.kind === "approval") { this.requestLazyDocument(APPROVAL_PAGE_ELEMENT); } + if (this.runtime.documentMode?.kind === "question") { + this.requestLazyDocument(QUESTION_PAGE_ELEMENT); + } const context = this.runtime.context; this.pendingGatewayUrl = this.runtime.pendingGatewayConnection?.gatewayUrl ?? null; // Context identity changes only across a full app-tree connection epoch; @@ -262,6 +266,16 @@ export class OpenClawApp extends OpenClawLightDomElement { return html``; } + private renderQuestionDocument(runtime: ApplicationRuntime) { + const lazyState = this.lazyCustomElements.visibleState; + if (lazyState?.element === QUESTION_PAGE_ELEMENT) { + return this.renderLazyDocumentState(QUESTION_PAGE_ELEMENT); + } + const questionId = + runtime.documentMode?.kind === "question" ? runtime.documentMode.questionId : ""; + return html``; + } + private replaceFocusDashboardLocation(location: RouteLocation, source: RouteLocation): void { const basePath = this.context?.basePath ?? ""; const expected = buildControlUiFocusPath( @@ -577,6 +591,13 @@ export class OpenClawApp extends OpenClawLightDomElement { `; } + if (runtime.documentMode?.kind === "question") { + return html` + + ${gatewayUrlConfirmation} ${this.renderQuestionDocument(runtime)} + + `; + } return html` diff --git a/ui/src/app/approval-deep-link.test.ts b/ui/src/app/approval-deep-link.test.ts index 26b76f4308fb..9d9df5d949d6 100644 --- a/ui/src/app/approval-deep-link.test.ts +++ b/ui/src/app/approval-deep-link.test.ts @@ -1,14 +1,15 @@ // @vitest-environment node import { describe, expect, it } from "vitest"; -import { resolveApprovalDocumentMode } from "./approval-deep-link.ts"; +import { inferBasePathFromPathname } from "../app-route-paths.ts"; +import { resolveControlUiDocumentMode } from "./approval-deep-link.ts"; -describe("resolveApprovalDocumentMode", () => { +describe("approval document routing", () => { it("resolves root and configured-base approval links", () => { - expect(resolveApprovalDocumentMode("/approve/exec%3A123", "")).toEqual({ + expect(resolveControlUiDocumentMode("/approve/exec%3A123", "")).toEqual({ kind: "approval", approvalId: "exec:123", }); - expect(resolveApprovalDocumentMode("/operator/approve/plugin%3A456", "/operator/")).toEqual({ + expect(resolveControlUiDocumentMode("/operator/approve/plugin%3A456", "/operator/")).toEqual({ kind: "approval", approvalId: "plugin:456", }); @@ -16,7 +17,7 @@ describe("resolveApprovalDocumentMode", () => { it("decodes one stable path segment without narrowing valid approval ids", () => { const approvalId = "plugin:a/b%🦞"; - expect(resolveApprovalDocumentMode(`/approve/${encodeURIComponent(approvalId)}`, "")).toEqual({ + expect(resolveControlUiDocumentMode(`/approve/${encodeURIComponent(approvalId)}`, "")).toEqual({ kind: "approval", approvalId, }); @@ -31,15 +32,49 @@ describe("resolveApprovalDocumentMode", () => { "/approve/id/extra", "/approve/id/", ])("keeps malformed approval-shaped paths shellless: %s", (pathname) => { - expect(resolveApprovalDocumentMode(pathname, "")).toEqual({ + expect(resolveControlUiDocumentMode(pathname, "")).toEqual({ kind: "approval", approvalId: null, }); }); it("does not claim ordinary or out-of-mount paths", () => { - expect(resolveApprovalDocumentMode("/chat", "")).toBeNull(); - expect(resolveApprovalDocumentMode("/approve/id", "/operator")).toBeNull(); - expect(resolveApprovalDocumentMode("/operator/approvals/id", "/operator")).toBeNull(); + expect(resolveControlUiDocumentMode("/chat", "")).toBeNull(); + expect(resolveControlUiDocumentMode("/approve/id", "/operator")).toBeNull(); + expect(resolveControlUiDocumentMode("/operator/approvals/id", "/operator")).toBeNull(); + }); +}); + +describe("question document routing", () => { + it("resolves root and configured-base question links without changing approval routing", () => { + expect(resolveControlUiDocumentMode("/ask/question%3A123", "")).toEqual({ + kind: "question", + questionId: "question:123", + }); + expect(resolveControlUiDocumentMode("/operator/ask/question%3A456", "/operator/")).toEqual({ + kind: "question", + questionId: "question:456", + }); + expect(resolveControlUiDocumentMode("/approve/approval%3A123", "")).toEqual({ + kind: "approval", + approvalId: "approval:123", + }); + expect(inferBasePathFromPathname("/ask/question%3A123")).toBe(""); + expect(inferBasePathFromPathname("/operator/ask/question%3A456")).toBe("/operator"); + }); + + it.each(["/ask", "/ask/", "/ask/%", "/ask/%2e", "/ask/id/extra", "/ask/id/"])( + "keeps malformed question-shaped paths shellless: %s", + (pathname) => { + expect(resolveControlUiDocumentMode(pathname, "")).toEqual({ + kind: "question", + questionId: null, + }); + }, + ); + + it("does not claim ordinary or out-of-mount paths", () => { + expect(resolveControlUiDocumentMode("/chat", "")).toBeNull(); + expect(resolveControlUiDocumentMode("/ask/id", "/operator")).toBeNull(); }); }); diff --git a/ui/src/app/approval-deep-link.ts b/ui/src/app/approval-deep-link.ts index 9725fa076de9..a318c9c3ef41 100644 --- a/ui/src/app/approval-deep-link.ts +++ b/ui/src/app/approval-deep-link.ts @@ -1,38 +1,78 @@ -import { normalizeBasePath } from "../app-route-paths.ts"; +import { CONTROL_UI_DOCUMENT_ROUTE_PATHS, normalizeBasePath } from "../app-route-paths.ts"; -export type ApprovalDocumentMode = { +type ApprovalDocumentMode = { kind: "approval"; approvalId: string | null; }; +type QuestionDocumentMode = { + kind: "question"; + questionId: string | null; +}; + +export type ControlUiDocumentMode = ApprovalDocumentMode | QuestionDocumentMode; + /** - * Recognizes the shellless approval document before the exact-path app router - * can replace it with Chat. The Gateway validates the decoded id; this parser - * only preserves the one-segment URL contract and rejects ambiguous paths. + * Recognizes shellless documents before the exact-path app router can replace + * them with Chat. Gateway owners validate decoded ids; this parser preserves + * the one-segment URL contract and rejects ambiguous paths. */ -export function resolveApprovalDocumentMode( +function resolveDocumentId( pathname: string, basePath: string, -): ApprovalDocumentMode | null { + routePath: string, +): string | null | undefined { const normalizedBasePath = normalizeBasePath(basePath); - const approvalRoot = `${normalizedBasePath}/approve`; - if (pathname === approvalRoot || pathname === `${approvalRoot}/`) { - return { kind: "approval", approvalId: null }; - } - const prefix = `${approvalRoot}/`; - if (!pathname.startsWith(prefix)) { + const documentRoot = `${normalizedBasePath}${routePath}`; + if (pathname === documentRoot || pathname === `${documentRoot}/`) { return null; } + const prefix = `${documentRoot}/`; + if (!pathname.startsWith(prefix)) { + return undefined; + } const encodedId = pathname.slice(prefix.length); if (!encodedId || encodedId.includes("/")) { - return { kind: "approval", approvalId: null }; + return null; } try { - const approvalId = decodeURIComponent(encodedId); - return approvalId && approvalId !== "." && approvalId !== ".." - ? { kind: "approval", approvalId } - : { kind: "approval", approvalId: null }; + const documentId = decodeURIComponent(encodedId); + return documentId && documentId !== "." && documentId !== ".." ? documentId : null; } catch { - return { kind: "approval", approvalId: null }; + return null; } } + +function resolveApprovalDocumentMode( + pathname: string, + basePath: string, +): ApprovalDocumentMode | null { + const approvalId = resolveDocumentId( + pathname, + basePath, + CONTROL_UI_DOCUMENT_ROUTE_PATHS.approval, + ); + return approvalId === undefined ? null : { kind: "approval", approvalId }; +} + +function resolveQuestionDocumentMode( + pathname: string, + basePath: string, +): QuestionDocumentMode | null { + const questionId = resolveDocumentId( + pathname, + basePath, + CONTROL_UI_DOCUMENT_ROUTE_PATHS.question, + ); + return questionId === undefined ? null : { kind: "question", questionId }; +} + +export function resolveControlUiDocumentMode( + pathname: string, + basePath: string, +): ControlUiDocumentMode | null { + return ( + resolveApprovalDocumentMode(pathname, basePath) ?? + resolveQuestionDocumentMode(pathname, basePath) + ); +} diff --git a/ui/src/app/bootstrap.ts b/ui/src/app/bootstrap.ts index f2c3b8bcc975..b33362921d56 100644 --- a/ui/src/app/bootstrap.ts +++ b/ui/src/app/bootstrap.ts @@ -33,7 +33,7 @@ import { } from "../pages/model-setup/first-run.ts"; import { createAgentSelectionCapability } from "./agent-selection.ts"; import { isBrowserPanelAvailable } from "./app-shell-chrome.ts"; -import { resolveApprovalDocumentMode, type ApprovalDocumentMode } from "./approval-deep-link.ts"; +import { resolveControlUiDocumentMode, type ControlUiDocumentMode } from "./approval-deep-link.ts"; import { createBrowserHistory, resolveControlUiPaths } from "./browser.ts"; import { createChatAttachmentHandoff } from "./chat-attachment-handoff.ts"; import { createApplicationConfigCapability } from "./config.ts"; @@ -238,7 +238,7 @@ function createApplicationNavigationPreferences( export type ApplicationRuntime = { readonly context: ApplicationContext; readonly router: ApplicationRouter; - readonly documentMode: ApprovalDocumentMode | null; + readonly documentMode: ControlUiDocumentMode | null; readonly focusLocation: ControlUiFocusLocation | null; readonly pendingGatewayConnection: { readonly gatewayUrl: string; @@ -268,7 +268,7 @@ export function bootstrapApplication( const [basePath, resourceBasePath] = resolveControlUiPaths( startupLocation.pathname || globalThis.location?.pathname || "/", ); - const documentMode = resolveApprovalDocumentMode(startupLocation.pathname, basePath); + const documentMode = resolveControlUiDocumentMode(startupLocation.pathname, basePath); const persistedSettings = loadSettings(); const initialSettings = documentMode ? resolvePageGatewaySettings(persistedSettings) diff --git a/ui/src/app/lazy-custom-element.ts b/ui/src/app/lazy-custom-element.ts index db3428683581..8f4473200a4f 100644 --- a/ui/src/app/lazy-custom-element.ts +++ b/ui/src/app/lazy-custom-element.ts @@ -282,6 +282,14 @@ export const APPROVAL_PAGE_ELEMENT = { loadModule: () => import("../pages/approval/approval-page-registration.ts"), } satisfies OptionalCustomElement; +const QUESTION_PAGE_TAG = "openclaw-question-page"; + +export const QUESTION_PAGE_ELEMENT = { + tagName: QUESTION_PAGE_TAG, + label: QUESTION_PAGE_TAG, + loadModule: () => import("../pages/question/question-page-registration.ts"), +} satisfies OptionalCustomElement; + // The card is in the chat graph, but modal-only queue controls stay off the // startup path until an approval is actually pending. const EXEC_APPROVAL_TAG = "openclaw-exec-approval"; diff --git a/ui/src/app/question-prompt-secret-store.test.ts b/ui/src/app/question-prompt-secret-store.test.ts new file mode 100644 index 000000000000..b1f8e1c69a04 --- /dev/null +++ b/ui/src/app/question-prompt-secret-store.test.ts @@ -0,0 +1,275 @@ +// @vitest-environment node +// Store-bound questions never retain submitted credentials in terminal UI state. +import { + DEFAULT_GATEWAY_REQUEST_TIMEOUT_MS, + type GatewayProtocolRequestOptions, +} from "@openclaw/gateway-client/browser"; +import type { Question, QuestionRecord, QuestionResolveResult } from "@openclaw/gateway-protocol"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { GatewayRequestError } from "../api/gateway.ts"; +import { waitForFast } from "../test-helpers/wait-for.ts"; +import { + createQuestionPromptState, + disposeQuestionPromptState, + handleQuestionPromptEvent, + refreshPendingQuestionsWithRetry, + setQuestionPromptClient, + submitQuestionPrompt, +} from "./question-prompt.ts"; + +type RequestFn = ( + method: string, + params?: unknown, + options?: GatewayProtocolRequestOptions, +) => Promise; +type SecretBinding = NonNullable; +type PromptState = ReturnType; + +const states: PromptState[] = []; +const storedAnswer = { + status: "answered", + answers: { answers: { secret_value: ["stored"] } }, +} satisfies QuestionResolveResult; + +function requestedSecret( + secretStore: SecretBinding = { name: "TEST_API_KEY", kind: "secret" }, + overrides: Partial = {}, +): QuestionRecord { + return { + id: "question-1", + questions: [ + { + questionId: "secret_value", + header: "API key", + question: "Provide the fake test credential.", + options: [], + isSecret: true, + secretStore, + ...overrides, + }, + ], + agentId: "main", + sessionKey: "agent:main:main", + createdAtMs: 1_000, + expiresAtMs: Date.now() + 60_000, + status: "pending", + }; +} + +function createSecretPrompt(request?: RequestFn, record = requestedSecret()) { + const state = createQuestionPromptState(vi.fn()); + states.push(state); + const client = request ? { request } : null; + if (client) { + setQuestionPromptClient(state, client); + } + handleQuestionPromptEvent(state, { event: "question.requested", payload: record }); + const prompt = state.prompts.get(record.id); + if (!prompt) { + throw new Error("valid secret question was not registered"); + } + return { state, prompt, client }; +} + +afterEach(() => { + for (const state of states.splice(0)) { + disposeQuestionPromptState(state); + } + vi.restoreAllMocks(); + vi.useRealTimers(); +}); + +describe("store-bound question normalization", () => { + it("preserves protocol-derived binding and replacement metadata", () => { + const secretStore: SecretBinding = { + name: "TEST_API_KEY", + kind: "secret", + allowedHosts: ["api.example.test"], + reason: "The test agent needs a fake credential.", + }; + const secretStoreExisting: NonNullable = { + updatedAtMs: 1_234, + updatedBy: "operator@example.test", + }; + const { prompt } = createSecretPrompt( + undefined, + requestedSecret(secretStore, { secretStoreExisting }), + ); + + expect(prompt.questions[0]).toMatchObject({ isSecret: true, secretStore, secretStoreExisting }); + }); + + it.each([ + { label: "non-boolean secrecy", overrides: { isSecret: "true" } }, + { + label: "duplicate hosts", + overrides: { + secretStore: { + name: "TEST_API_KEY", + kind: "secret", + allowedHosts: ["api.example.test", "api.example.test"], + }, + }, + }, + { + label: "hosts on environment entries", + overrides: { + secretStore: { + name: "TEST_ENV_VALUE", + kind: "env", + allowedHosts: ["api.example.test"], + }, + }, + }, + { + label: "invalid replacement metadata", + overrides: { secretStoreExisting: { updatedAtMs: -1 } }, + }, + ])("rejects $label", ({ overrides }) => { + const state = createQuestionPromptState(vi.fn()); + states.push(state); + const record = requestedSecret(); + + expect( + handleQuestionPromptEvent(state, { + event: "question.requested", + payload: { ...record, questions: [{ ...record.questions[0], ...overrides }] }, + }), + ).toBe(false); + }); +}); + +describe("store-bound question submission", () => { + it.each([ + { + label: "untouched proposed hosts", + binding: { name: "TEST_API_KEY", kind: "secret", allowedHosts: ["proposed.example.test"] }, + draft: undefined, + expectedHosts: ["proposed.example.test"], + }, + { + label: "edited comma-and-whitespace-separated hosts", + binding: { name: "TEST_API_KEY", kind: "secret" }, + draft: " first.example.test, second.example.test\nthird.example.test ", + expectedHosts: ["first.example.test", "second.example.test", "third.example.test"], + }, + { + label: "an explicitly cleared host proposal", + binding: { name: "TEST_API_KEY", kind: "secret", allowedHosts: ["proposed.example.test"] }, + draft: " ", + expectedHosts: [], + }, + { + label: "an untouched secret without proposed hosts", + binding: { name: "TEST_API_KEY", kind: "secret" }, + draft: undefined, + expectedHosts: undefined, + }, + { + label: "an environment entry despite stale host input", + binding: { name: "TEST_ENV_VALUE", kind: "env" }, + draft: "ignored.example.test", + expectedHosts: undefined, + }, + ] satisfies Array<{ + label: string; + binding: SecretBinding; + draft: string | undefined; + expectedHosts: string[] | undefined; + }>)("submits $label", async ({ binding, draft, expectedHosts }) => { + const request = vi.fn(async () => storedAnswer); + const { state, prompt } = createSecretPrompt(request, requestedSecret(binding)); + if (draft !== undefined) { + prompt.secretStoreAllowedHostsDraft = draft; + } + + await submitQuestionPrompt(state, prompt.id, { secret_value: ["fake-secret-test-value"] }); + + expect(request).toHaveBeenCalledWith( + "question.resolve", + { + id: prompt.id, + answers: { answers: { secret_value: ["fake-secret-test-value"] } }, + ...(expectedHosts !== undefined ? { secretStoreAllowedHosts: expectedHosts } : {}), + }, + { timeoutMs: DEFAULT_GATEWAY_REQUEST_TIMEOUT_MS }, + ); + expect(prompt.submittedAnswers).toEqual(storedAnswer.answers); + }); + + it("preserves a secret draft on validation failure and clears it after successful retry", async () => { + const fakeSecret = "fake-secret-retry-test-value"; + const request = vi + .fn() + .mockRejectedValueOnce( + new GatewayRequestError({ code: "INVALID_REQUEST", message: "Allowed host is invalid." }), + ) + .mockResolvedValueOnce(storedAnswer); + const { state, prompt } = createSecretPrompt(request); + prompt.drafts.set("secret_value", { selected: new Set(), freeText: fakeSecret }); + + await submitQuestionPrompt(state, prompt.id, { secret_value: [fakeSecret] }); + + expect(prompt).toMatchObject({ + status: "pending", + submitting: false, + error: "Allowed host is invalid.", + submittedAnswers: storedAnswer.answers, + }); + expect(prompt.drafts.get("secret_value")?.freeText).toBe(fakeSecret); + + await submitQuestionPrompt(state, prompt.id, { secret_value: [fakeSecret] }); + + expect(prompt).toMatchObject({ + status: "answered", + answers: storedAnswer.answers, + submittedAnswers: storedAnswer.answers, + }); + expect(prompt.drafts.has("secret_value")).toBe(false); + }); +}); + +describe("store-bound question terminal cleanup", () => { + it("forgets the secret draft when the local deadline expires", () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-07-17T00:00:00.000Z")); + const record = requestedSecret(); + record.expiresAtMs = Date.now() + 1_000; + const { prompt } = createSecretPrompt(undefined, record); + prompt.drafts.set("secret_value", { + selected: new Set(), + freeText: "fake-secret-expired-test-value", + }); + + vi.advanceTimersByTime(1_000); + + expect(prompt.status).toBe("expired"); + expect(prompt.drafts.has("secret_value")).toBe(false); + }); + + it("forgets the secret draft when recovery proves the question unavailable", async () => { + const request = vi.fn(async (method) => { + if (method === "question.list") { + return { questions: [] }; + } + throw new GatewayRequestError({ + code: "INVALID_REQUEST", + message: "question was not found", + details: { reason: "QUESTION_NOT_FOUND" }, + }); + }); + const { state, prompt, client } = createSecretPrompt(request); + prompt.drafts.set("secret_value", { + selected: new Set(), + freeText: "fake-secret-unavailable-test-value", + }); + if (!client) { + throw new Error("connected secret question has no client"); + } + + refreshPendingQuestionsWithRetry(state, client); + await waitForFast(() => expect(prompt.status).toBe("unavailable")); + + expect(prompt.drafts.has("secret_value")).toBe(false); + }); +}); diff --git a/ui/src/app/question-prompt-secret-store.ts b/ui/src/app/question-prompt-secret-store.ts new file mode 100644 index 000000000000..9f20a2a7a5e5 --- /dev/null +++ b/ui/src/app/question-prompt-secret-store.ts @@ -0,0 +1,148 @@ +// Store-bound question ownership keeps credential metadata and plaintext handling together. +import { asSafeIntegerInRange } from "@openclaw/normalization-core/number-coercion"; +import { isRecord } from "@openclaw/normalization-core/record-coerce"; +import { normalizeNullableString } from "@openclaw/normalization-core/string-coerce"; +import type { + Question, + QuestionAnswers, + QuestionResolveParams, + QuestionResolveResult, +} from "../../../packages/gateway-protocol/src/index.js"; + +type QuestionSecretStoreFields = Pick; + +function parseQuestionSecretStore(value: unknown): NonNullable | null { + if (!isRecord(value)) { + return null; + } + const { name, kind, allowedHosts, reason } = value; + if ( + typeof name !== "string" || + !/^[A-Z][A-Z0-9_]{0,127}$/.test(name) || + (kind !== "secret" && kind !== "env") || + (reason !== undefined && (typeof reason !== "string" || reason.length > 200)) + ) { + return null; + } + if ( + allowedHosts !== undefined && + (kind !== "secret" || + !Array.isArray(allowedHosts) || + allowedHosts.length > 128 || + allowedHosts.some((host) => typeof host !== "string" || !host || host.length > 253) || + new Set(allowedHosts).size !== allowedHosts.length) + ) { + return null; + } + return { + name, + kind, + ...(allowedHosts !== undefined ? { allowedHosts: [...allowedHosts] } : {}), + ...(reason !== undefined ? { reason } : {}), + }; +} + +function parseQuestionSecretStoreExisting( + value: unknown, +): NonNullable | null { + if (!isRecord(value)) { + return null; + } + const updatedAtMs = asSafeIntegerInRange(value.updatedAtMs, { min: 0 }); + const updatedBy = + value.updatedBy === undefined ? undefined : normalizeNullableString(value.updatedBy); + if (updatedAtMs === undefined || (value.updatedBy !== undefined && !updatedBy)) { + return null; + } + return { updatedAtMs, ...(updatedBy ? { updatedBy } : {}) }; +} + +export function normalizeQuestionSecretStoreFields( + value: Record, +): QuestionSecretStoreFields | null { + if (value.isSecret !== undefined && typeof value.isSecret !== "boolean") { + return null; + } + const secretStore = + value.secretStore === undefined ? undefined : parseQuestionSecretStore(value.secretStore); + const secretStoreExisting = + value.secretStoreExisting === undefined + ? undefined + : parseQuestionSecretStoreExisting(value.secretStoreExisting); + if ( + secretStore === null || + secretStoreExisting === null || + (secretStore && !value.isSecret) || + (secretStoreExisting && !secretStore) + ) { + return null; + } + return { + ...(typeof value.isSecret === "boolean" ? { isSecret: value.isSecret } : {}), + ...(secretStore ? { secretStore } : {}), + ...(secretStoreExisting ? { secretStoreExisting } : {}), + }; +} + +export function clearSecretQuestionDrafts( + questions: readonly Question[], + drafts: { delete: (questionId: string) => boolean }, +): void { + for (const question of questions) { + if (question.isSecret) { + drafts.delete(question.questionId); + } + } +} + +export function parseQuestionSubmissionResult( + value: unknown, + parseAnswers: (value: unknown) => QuestionAnswers | null, +): QuestionResolveResult | null { + if (!isRecord(value)) { + return null; + } + if (value.status === "cancelled") { + return { status: "cancelled" }; + } + const answers = parseAnswers(value.answers); + return value.status === "answered" && answers ? { status: "answered", answers } : null; +} + +export function prepareQuestionSecretStoreSubmission( + id: string, + questions: readonly Question[], + resolution: { answers: QuestionAnswers["answers"] } | { cancel: true }, + allowedHostsDraft?: string, +): { requestParams: QuestionResolveParams; submittedAnswers?: QuestionAnswers } { + if (!("answers" in resolution)) { + return { requestParams: { id, cancel: true } }; + } + const requestAnswers: QuestionAnswers = { + answers: Object.fromEntries( + Object.entries(resolution.answers).map(([questionId, answers]) => [questionId, [...answers]]), + ), + }; + const secretQuestion = questions[0]?.secretStore ? questions[0] : undefined; + const allowedHosts = + secretQuestion?.secretStore?.kind === "secret" + ? allowedHostsDraft !== undefined + ? allowedHostsDraft + .split(/[,\s]+/u) + .map((host) => host.trim()) + .filter(Boolean) + : secretQuestion.secretStore.allowedHosts + : undefined; + return { + requestParams: { + id, + answers: requestAnswers, + ...(allowedHosts !== undefined ? { secretStoreAllowedHosts: allowedHosts } : {}), + }, + // The Gateway owns redaction: retained prompt state receives its synthetic + // marker while plaintext remains only in the outbound request payload. + submittedAnswers: secretQuestion + ? { answers: { [secretQuestion.questionId]: ["stored"] } } + : requestAnswers, + }; +} diff --git a/ui/src/app/question-prompt.ts b/ui/src/app/question-prompt.ts index 54c3a6c301d6..fd876d8a74a6 100644 --- a/ui/src/app/question-prompt.ts +++ b/ui/src/app/question-prompt.ts @@ -6,7 +6,6 @@ import type { Question, QuestionAnswers, QuestionRecord, - QuestionResolveResult, QuestionResolvedEvent, } from "../../../packages/gateway-protocol/src/index.js"; import { GatewayRequestError, type GatewayEventFrame } from "../api/gateway.ts"; @@ -20,6 +19,12 @@ import { type QuestionClient, type QuestionClientResolutionOwner, } from "./question-prompt-client.ts"; +import { + clearSecretQuestionDrafts, + normalizeQuestionSecretStoreFields, + parseQuestionSubmissionResult, + prepareQuestionSecretStoreSubmission, +} from "./question-prompt-secret-store.ts"; type QuestionDraft = { selected: Set; @@ -45,6 +50,7 @@ export type QuestionPrompt = { submitting: boolean; error: string | null; drafts: Map; + secretStoreAllowedHostsDraft?: string; revision: number; }; @@ -119,6 +125,10 @@ function parseQuestion(value: unknown): Question | null { return null; } } + const secretStoreFields = normalizeQuestionSecretStoreFields(value); + if (!secretStoreFields) { + return null; + } return { questionId, header: clampedHeader, @@ -126,6 +136,7 @@ function parseQuestion(value: unknown): Question | null { options, ...(value.multiSelect === true ? { multiSelect: true } : {}), ...(typeof value.isOther === "boolean" ? { isOther: value.isOther } : {}), + ...secretStoreFields, }; } @@ -245,17 +256,6 @@ function parseQuestionResolvedEvent(payload: unknown): QuestionResolvedEvent | n return null; } -function parseQuestionResolveResult(payload: unknown): QuestionResolveResult | null { - if (!isRecord(payload)) { - return null; - } - if (payload.status === "cancelled") { - return { status: "cancelled" }; - } - const answers = parseQuestionAnswers(payload.answers); - return payload.status === "answered" && answers ? { status: "answered", answers } : null; -} - export function createQuestionPromptState(onChange: () => void): QuestionPromptState { const state: QuestionPromptState = { client: null, @@ -286,6 +286,7 @@ function scheduleTick(state: QuestionPromptState): void { for (const prompt of state.prompts.values()) { if (prompt.status === "pending" && prompt.expiresAtMs <= now) { prompt.status = "expired"; + clearSecretQuestionDrafts(prompt.questions, prompt.drafts); prompt.locallyExpired = true; prompt.submitting = false; prompt.error = null; @@ -306,6 +307,10 @@ function promptFromRecord( previous?: QuestionPrompt, ): QuestionPrompt { const revision = ++state.revision; + const drafts = previous?.drafts ?? new Map(); + if (record.status !== "pending") { + clearSecretQuestionDrafts(record.questions, drafts); + } return { id: record.id, questions: record.questions, @@ -329,7 +334,10 @@ function promptFromRecord( ? (previous?.submitting ?? false) : false, error: record.status === "pending" ? (previous?.error ?? null) : null, - drafts: previous?.drafts ?? new Map(), + drafts, + ...(previous?.secretStoreAllowedHostsDraft !== undefined + ? { secretStoreAllowedHostsDraft: previous.secretStoreAllowedHostsDraft } + : {}), revision, }; } @@ -340,6 +348,7 @@ function applyQuestionResolution( resolved: QuestionResolvedEvent, ): void { prompt.status = resolved.status; + clearSecretQuestionDrafts(prompt.questions, prompt.drafts); prompt.answers = resolved.status === "answered" ? resolved.answers : undefined; const matchesSubmittedAnswer = resolved.status === "answered" && @@ -433,6 +442,7 @@ function markRecoveryUnavailable(state: QuestionPromptState, prompt: QuestionPro // QUESTION_NOT_FOUND means the gateway tombstone aged out. It proves the prompt is // no longer actionable, but not whether it was answered, cancelled, or expired. prompt.status = "unavailable"; + clearSecretQuestionDrafts(prompt.questions, prompt.drafts); prompt.answers = undefined; prompt.answeredElsewhere = false; prompt.localResolutionConfirmed = false; @@ -656,12 +666,6 @@ export function disposeQuestionPromptState(state: QuestionPromptState): void { state.client = null; } -function buildAnswers(values: QuestionAnswerValues): QuestionAnswers { - return { - answers: Object.fromEntries(Object.entries(values).map(([id, answers]) => [id, [...answers]])), - }; -} - async function resolveQuestionPrompt( state: QuestionPromptState, id: string, @@ -680,8 +684,13 @@ async function resolveQuestionPrompt( return; } prompt.submitting = true; - const submittedAnswers = "answers" in resolution ? buildAnswers(resolution.answers) : undefined; - prompt.submittedAnswers = submittedAnswers; + const submission = prepareQuestionSecretStoreSubmission( + id, + prompt.questions, + resolution, + prompt.secretStoreAllowedHostsDraft, + ); + prompt.submittedAnswers = submission.submittedAnswers; prompt.error = null; prompt.revision = ++state.revision; state.onChange(); @@ -689,11 +698,11 @@ async function resolveQuestionPrompt( const result = await requestQuestionGateway( client, "question.resolve", - submittedAnswers ? { id, answers: submittedAnswers } : { id, cancel: true }, + submission.requestParams, prompt.expiresAtMs, ); - const resolved = parseQuestionResolveResult(result); - if (!resolved || resolved.status !== (submittedAnswers ? "answered" : "cancelled")) { + const resolved = parseQuestionSubmissionResult(result, parseQuestionAnswers); + if (!resolved || resolved.status !== (submission.submittedAnswers ? "answered" : "cancelled")) { throw new Error("invalid question.resolve response"); } if (state.client === client && state.clientGeneration === clientGeneration) { diff --git a/ui/src/e2e/question-flow.e2e.test.ts b/ui/src/e2e/question-flow.e2e.test.ts index 9357bf8244ea..81819bb1f801 100644 --- a/ui/src/e2e/question-flow.e2e.test.ts +++ b/ui/src/e2e/question-flow.e2e.test.ts @@ -1,7 +1,7 @@ // Control UI E2E tests cover composer-replacing Gateway questions through the mocked WebSocket. import { mkdir } from "node:fs/promises"; import path from "node:path"; -import type { QuestionResolveResult } from "@openclaw/gateway-protocol"; +import type { Question, QuestionResolveResult } from "@openclaw/gateway-protocol"; import type { BrowserContext, Page } from "playwright"; import { afterEach, expect, it } from "vitest"; import type { SessionsListResult } from "../api/types.ts"; @@ -27,17 +27,7 @@ const mainSessionKey = "agent:main:main"; const questionSessionKey = "agent:main:question-proof"; let context: BrowserContext | undefined; -function questionRecord( - id: string, - questions: Array<{ - questionId: string; - header: string; - question: string; - options: Array<{ label: string; description?: string }>; - multiSelect?: boolean; - isOther?: boolean; - }>, -) { +function questionRecord(id: string, questions: Question[]) { const createdAtMs = Date.now(); return { id, @@ -50,6 +40,32 @@ function questionRecord( }; } +function secretStoreQuestion(id: string) { + return questionRecord(id, [ + { + questionId: "api_key", + header: "API key", + question: "Provide the deployment API key", + options: [], + isSecret: true, + secretStore: { + name: "DEPLOY_API_KEY", + kind: "secret", + allowedHosts: ["api.example.test"], + reason: "Publish the release artifacts.", + }, + secretStoreExisting: { + updatedAtMs: Date.now() - 60_000, + updatedBy: "release-operator", + }, + }, + ]); +} + +function storedSecretAnswer(): QuestionResolveResult { + return { status: "answered", answers: { answers: { api_key: ["stored"] } } }; +} + async function screenshot(page: Page, name: string) { if (!captureUiProof) { return; @@ -347,6 +363,143 @@ suite.define(() => { await screenshot(page, "02-question-answered.png"); }); + it("masks a store-bound secret and resolves it with edited hosts without echoing the value", async () => { + const { gateway, page } = await openQuestionPage(); + const request = secretStoreQuestion("question-store-secret-success"); + const fakeSecret = "fake-secret-never-use-browser-proof-123"; + await gateway.setMethodResponse("question.resolve", storedSecretAnswer()); + await emitRequested(gateway, request); + + const panel = panelFor(page, "Provide the deployment API key"); + await panel.waitFor(); + await expect.poll(() => panel.getByText("Requested by main", { exact: false }).count()).toBe(1); + await expect.poll(() => panel.getByText("Publish the release artifacts.").count()).toBe(1); + await expect + .poll(() => panel.getByText("Replaces DEPLOY_API_KEY", { exact: false }).count()) + .toBe(1); + const secretInput = panel.locator('input[type="password"]'); + await expect.poll(() => secretInput.count()).toBe(1); + expect(await secretInput.getAttribute("autocomplete")).toBe("off"); + const hostsInput = panel.locator(".chat-question-panel__hosts"); + expect(await hostsInput.inputValue()).toBe("api.example.test"); + await screenshot(page, "07-secret-store-pending.png"); + + await secretInput.fill(fakeSecret); + await hostsInput.fill("api.example.test, uploads.example.test extra.example.test"); + expect(await page.locator("body").textContent()).not.toContain(fakeSecret); + expect(await page.locator("body").evaluate((element) => element.innerHTML)).not.toContain( + fakeSecret, + ); + await screenshot(page, "08-secret-store-masked-input.png"); + + await panel.getByRole("button", { name: "Submit", exact: true }).click(); + const resolveRequest = await gateway.waitForRequest("question.resolve"); + expect(resolveRequest.params).toEqual({ + id: request.id, + answers: { answers: { api_key: [fakeSecret] } }, + secretStoreAllowedHosts: ["api.example.test", "uploads.example.test", "extra.example.test"], + }); + await expect.poll(() => panel.count()).toBe(0); + const summary = page.locator(".chat-question-summary").filter({ hasText: "API key:" }); + await summary.waitFor(); + await expect.poll(() => summary.getByText("Answered", { exact: true }).count()).toBe(1); + expect(await summary.textContent()).not.toContain("stored"); + expect(await page.locator("body").textContent()).not.toContain(fakeSecret); + await screenshot(page, "09-secret-store-answered.png"); + }); + + it("keeps a store-bound question interactive after Gateway validation rejects its hosts", async () => { + const { gateway, page } = await openQuestionPage(); + const request = secretStoreQuestion("question-store-secret-validation"); + const fakeSecret = "fake-secret-never-use-validation-proof-456"; + const validationMessage = "Allowed hosts must be valid hostnames."; + await gateway.setMethodResponse("question.resolve", { + __mockError: { code: "INVALID_REQUEST", message: validationMessage }, + }); + await emitRequested(gateway, request); + + const panel = panelFor(page, "Provide the deployment API key"); + const secretInput = panel.locator('input[type="password"]'); + const hostsInput = panel.locator(".chat-question-panel__hosts"); + await secretInput.fill(fakeSecret); + await hostsInput.fill("bad-host.example.test"); + await panel.getByRole("button", { name: "Submit", exact: true }).click(); + await gateway.waitForRequest("question.resolve"); + await panel.getByText(validationMessage, { exact: false }).waitFor(); + await expect.poll(() => secretInput.isEnabled()).toBe(true); + expect(await secretInput.inputValue()).toBe(fakeSecret); + expect(await page.locator("body").textContent()).not.toContain(fakeSecret); + await screenshot(page, "10-secret-store-validation-error.png"); + + await gateway.setMethodResponse("question.resolve", storedSecretAnswer()); + await hostsInput.fill("corrected.example.test"); + const previousRequestCount = (await gateway.getRequests("question.resolve")).length; + await panel.getByRole("button", { name: "Submit", exact: true }).click(); + const retry = await gateway.waitForRequest("question.resolve", { + after: previousRequestCount, + }); + expect(retry.params).toEqual({ + id: request.id, + answers: { answers: { api_key: [fakeSecret] } }, + secretStoreAllowedHosts: ["corrected.example.test"], + }); + await expect.poll(() => panel.count()).toBe(0); + expect(await page.locator("body").textContent()).not.toContain(fakeSecret); + }); + + it("loads a mounted /ask document and resolves its secret through the shared question card", async () => { + context = await suite.browser.newContext({ + locale: "en-US", + serviceWorkers: "block", + viewport: { height: 900, width: 1440 }, + }); + const page = await context.newPage(); + const request = secretStoreQuestion("question-store-secret-deep-link"); + const fakeSecret = "fake-secret-never-use-deep-link-proof-789"; + const gateway = await installMockGateway(page, { + basePath: "/operator", + featureMethods: ["question.get", "question.resolve"], + methodResponses: { + "question.get": { question: request }, + "question.resolve": storedSecretAnswer(), + }, + }); + const documentUrl = new URL( + `operator/ask/${encodeURIComponent(request.id)}`, + suite.server.baseUrl, + ); + await page.goto(documentUrl.toString()); + const getRequest = await gateway.waitForRequest("question.get"); + expect(getRequest.params).toEqual({ id: request.id }); + + const document = page.locator("openclaw-question-page"); + await document.waitFor(); + expect(await page.locator("openclaw-app-shell, openclaw-app-sidebar").count()).toBe(0); + const panel = document.locator("openclaw-chat-question-panel"); + await panel.waitFor(); + await screenshot(page, "11-secret-store-ask-pending.png"); + const secretInput = panel.locator('input[type="password"]'); + await secretInput.fill(fakeSecret); + expect(await page.locator("body").textContent()).not.toContain(fakeSecret); + expect(await page.locator("body").evaluate((element) => element.innerHTML)).not.toContain( + fakeSecret, + ); + await screenshot(page, "12-secret-store-ask-masked-input.png"); + await panel.getByRole("button", { name: "Submit", exact: true }).click(); + + const resolveRequest = await gateway.waitForRequest("question.resolve"); + expect(resolveRequest.params).toEqual({ + id: request.id, + answers: { answers: { api_key: [fakeSecret] } }, + secretStoreAllowedHosts: ["api.example.test"], + }); + await document.getByRole("heading", { name: "Answered", exact: true }).waitFor(); + expect(await document.textContent()).not.toContain(fakeSecret); + expect(await document.textContent()).not.toContain("stored"); + expect(new URL(page.url()).pathname).toBe(`/operator/ask/${request.id}`); + await screenshot(page, "13-secret-store-ask-answered.png"); + }); + it("keeps multi-select on one step and submits labels as an array", async () => { const { gateway, page } = await openQuestionPage(); const request = questionRecord("question-release-checks", [ diff --git a/ui/src/i18n/locales/en.ts b/ui/src/i18n/locales/en.ts index b971fd1c2bfc..d2f599946495 100644 --- a/ui/src/i18n/locales/en.ts +++ b/ui/src/i18n/locales/en.ts @@ -5705,6 +5705,10 @@ export const en: TranslationMap = { eyebrow: "Question", summaryLabel: "Question outcome", ownAnswerFor: "Your own answer for {header}", + storeRequestedBy: "Requested by {agent} · {session}", + storeEntry: "Stores {name} as {kind}", + storeReplacement: "Replaces {name} — last updated {updated}", + storeReplacementBy: "Replaces {name} — last updated {updated} by {updatedBy}", submitting: "Submitting…", submitFailed: "Could not submit: {error}", answered: "Answered", diff --git a/ui/src/pages/chat/chat-thread.question.test.ts b/ui/src/pages/chat/chat-thread.question.test.ts index 8c79a2520144..a6e1222ec34f 100644 --- a/ui/src/pages/chat/chat-thread.question.test.ts +++ b/ui/src/pages/chat/chat-thread.question.test.ts @@ -121,6 +121,29 @@ describe("question chat items", () => { ).toContain("Format: Detailed"); }); + it("never echoes a secret answer in the terminal transcript summary", () => { + const answered = prompt("answered"); + answered.questions = [ + { + questionId: "api_key", + header: "API key", + question: "Provide the deployment API key", + options: [], + isSecret: true, + secretStore: { name: "FAKE_DEPLOYMENT_API_KEY", kind: "secret" }, + }, + ]; + answered.answeredElsewhere = true; + answered.answers = { answers: { api_key: ["fake-secret-never-render"] } }; + const container = document.createElement("div"); + + render(renderChatQuestionSummary(answered), container); + + expect(container.textContent?.replace(/\s+/g, " ")).toContain("API key: Answered"); + expect(container.textContent).not.toContain("fake-secret-never-render"); + expect(container.innerHTML).not.toContain("fake-secret-never-render"); + }); + it("omits questions belonging to another session", () => { const other = prompt("pending"); other.sessionKey = "agent:other:main"; diff --git a/ui/src/pages/chat/components/chat-question-card.test.ts b/ui/src/pages/chat/components/chat-question-card.test.ts index 90e331ae99ae..b284639aad04 100644 --- a/ui/src/pages/chat/components/chat-question-card.test.ts +++ b/ui/src/pages/chat/components/chat-question-card.test.ts @@ -138,6 +138,84 @@ describe("shared question panel", () => { }); }); + it("keeps a store-bound secret masked while preserving editable destination hosts", async () => { + const prompt = gatewayPrompt({ + agentId: "release-agent", + questions: [ + { + questionId: "api_key", + header: "API key", + question: "Provide the deployment API key", + options: [], + isSecret: true, + secretStore: { + name: "FAKE_DEPLOYMENT_API_KEY", + kind: "secret", + allowedHosts: ["api.example.test"], + reason: "Deploy the approved release", + }, + secretStoreExisting: { + updatedAtMs: Date.now() - 60_000, + updatedBy: "release-owner", + }, + }, + ], + }); + const onSubmit = vi.fn(); + drawGateway(prompt, { onSubmit }); + const panel = await panelIn(container); + const hosts = container.querySelector(".chat-question-panel__hosts")!; + const secret = container.querySelector('input[type="password"]')!; + + expect(hosts.value).toBe("api.example.test"); + expect(secret.autocomplete).toBe("off"); + expect(container.textContent).toContain("release-agent"); + expect(container.textContent).toContain("agent:main:main"); + expect(container.textContent).toContain("Stores FAKE_DEPLOYMENT_API_KEY as Protected secret"); + expect(container.textContent).toContain("Deploy the approved release"); + expect(container.textContent).toContain("Replaces FAKE_DEPLOYMENT_API_KEY — last updated"); + expect(container.textContent).toContain("by release-owner"); + + hosts.value = "api.example.test, uploads.example.test"; + hosts.dispatchEvent(new InputEvent("input", { bubbles: true })); + await panel.updateComplete; + expect(prompt.secretStoreAllowedHostsDraft).toBe("api.example.test, uploads.example.test"); + + const fakeSecret = "fake-secret-value-for-ui-test"; + secret.value = fakeSecret; + secret.dispatchEvent(new InputEvent("input", { bubbles: true })); + await panel.updateComplete; + expect(container.textContent).not.toContain(fakeSecret); + expect(container.innerHTML).not.toContain(fakeSecret); + + container.querySelector(".chat-question-panel__advance")?.click(); + expect(onSubmit).toHaveBeenCalledWith({ api_key: [fakeSecret] }); + }); + + it("keeps environment store requests masked without exposing a destination-host editor", async () => { + drawGateway( + gatewayPrompt({ + questions: [ + { + questionId: "environment_value", + header: "Environment", + question: "Provide the environment value", + options: [], + isSecret: true, + secretStore: { name: "FAKE_ENVIRONMENT_VALUE", kind: "env" }, + }, + ], + }), + ); + await panelIn(container); + + expect(container.querySelector('input[type="password"]')).not.toBeNull(); + expect(container.querySelector(".chat-question-panel__hosts")).toBeNull(); + expect(container.textContent).toContain( + "Stores FAKE_ENVIRONMENT_VALUE as Agent-readable environment", + ); + }); + it("supports numeric selection and Enter submission while focused", async () => { const onSubmit = vi.fn(); drawGateway(gatewayPrompt(), { onSubmit }); diff --git a/ui/src/pages/chat/components/chat-question-card.ts b/ui/src/pages/chat/components/chat-question-card.ts index 2caf7874a415..24defe8ef63c 100644 --- a/ui/src/pages/chat/components/chat-question-card.ts +++ b/ui/src/pages/chat/components/chat-question-card.ts @@ -4,20 +4,17 @@ import { property, state } from "lit/decorators.js"; import type { QuestionPrompt } from "../../../app/question-prompt.ts"; import { icons } from "../../../components/icons.ts"; import { t } from "../../../i18n/index.ts"; +import { formatRelativeTimestamp } from "../../../lib/format.ts"; -type QuestionPanelQuestion = { - questionId: string; - header: string; - question: string; - options: Array<{ label: string; description?: string }>; - multiSelect?: boolean; - isOther?: boolean; -}; +type QuestionPanelQuestion = QuestionPrompt["questions"][number]; type QuestionPanelViewModel = { requestKey: string; title: string; questions: QuestionPanelQuestion[]; + agentId?: string; + sessionKey?: string; + secretStoreAllowedHostsDraft?: string; collapsed: boolean; disabled: boolean; submitting?: boolean; @@ -31,6 +28,7 @@ type QuestionPanelProps = { onSubmit?: (answersById: Record) => void | Promise; onSkip?: () => void | Promise; onAnswersChange?: (answersById: Record) => void; + onSecretStoreAllowedHostsChange?: (allowedHosts: string) => void; onDismissError?: () => void; onCollapsedChange?: (collapsed: boolean) => void; onPreviousRequest?: () => void; @@ -80,6 +78,9 @@ export function createGatewayQuestionPanelProps( requestKey: prompt.id, title: t("chat.questions.eyebrow"), questions: prompt.questions, + agentId: prompt.agentId, + sessionKey: prompt.sessionKey, + secretStoreAllowedHostsDraft: prompt.secretStoreAllowedHostsDraft, collapsed: options.collapsed ?? false, disabled: prompt.status !== "pending" || prompt.submitting, submitting: prompt.submitting, @@ -91,6 +92,10 @@ export function createGatewayQuestionPanelProps( updatePromptDrafts(prompt, answersById); options.onChange?.(); }, + onSecretStoreAllowedHostsChange: (allowedHosts) => { + prompt.secretStoreAllowedHostsDraft = allowedHosts; + options.onChange?.(); + }, onSubmit: options.onSubmit ? async (answersById) => { await options.onSubmit?.(answersById); @@ -130,6 +135,9 @@ function terminalAnswer(prompt: QuestionPrompt, question: QuestionPanelQuestion) if (prompt.status === "unavailable") { return t("chat.questions.unavailable"); } + if (question.isSecret) { + return t("chat.questions.answered"); + } const answer = prompt.answers?.answers[question.questionId]?.join(", "); if (answer) { return answer; @@ -577,6 +585,71 @@ class ChatQuestionPanel extends LitElement { })} + ${question.secretStore + ? html` +
+
+ ${t("chat.questions.storeRequestedBy", { + agent: model.agentId ?? t("common.unknown"), + session: model.sessionKey ?? t("common.unknown"), + })} +
+
+ ${t("chat.questions.storeEntry", { + name: question.secretStore.name, + kind: + question.secretStore.kind === "secret" + ? t("secretsStore.protectedSecret") + : t("secretsStore.agentReadable"), + })} +
+ ${question.secretStore.reason + ? html`
+ ${question.secretStore.reason} +
` + : nothing} + ${question.secretStoreExisting + ? html`
+ ${question.secretStoreExisting.updatedBy + ? t("chat.questions.storeReplacementBy", { + name: question.secretStore.name, + updated: formatRelativeTimestamp( + question.secretStoreExisting.updatedAtMs, + ), + updatedBy: question.secretStoreExisting.updatedBy, + }) + : t("chat.questions.storeReplacement", { + name: question.secretStore.name, + updated: formatRelativeTimestamp( + question.secretStoreExisting.updatedAtMs, + ), + })} +
` + : nothing} + ${question.secretStore.kind === "secret" + ? html`` + : nothing} +
+ ` + : nothing} ${question.isOther || question.options.length === 0 ? html`