Skip to content

Fix Talk SecretRef config payloads for native clients#91020

Closed
joshp123 wants to merge 1 commit into
mainfrom
fix/talk-secretref-ios-config
Closed

Fix Talk SecretRef config payloads for native clients#91020
joshp123 wants to merge 1 commit into
mainfrom
fix/talk-secretref-ios-config

Conversation

@joshp123

@joshp123 joshp123 commented Jun 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Fix talk.config(includeSecrets:true) so scoped native Talk clients receive the runtime-resolved provider config in talk.resolved.config.
  • Preserve read-scope safety: source SecretRef context remains visible as SecretRef shape and secret-bearing fields stay redacted without operator.talk.secrets.
  • Add focused gateway protocol and iOS parser regressions for the native ElevenLabs Talk path.

Minimality

Production delta is one resolver branch in src/gateway/server-methods/talk.ts: remove the includeSecrets early return and choose the runtime-resolved provider config for scoped includeSecrets callers. No iOS runtime code changed; the iOS diff is parser coverage only.

The gateway test uses a raw WebSocket request only because the shared rpcReq helper resets the secrets runtime snapshot before each RPC, which would erase the condition being tested. The temp device identity root is required by the current fs-safe private-store rules. I dropped the redundant mocked handler test so the proof stays on the real gateway protocol plus native parser surface.

Problem

Native Talk docs describe talk.providers.elevenlabs.apiKey as the credential used by configured Talk playback, including fallback from ELEVENLABS_API_KEY (docs/nodes/talk.md:64, docs/nodes/talk.md:111). The secrets runtime already resolves talk.providers.*.apiKey into the active runtime snapshot (src/secrets/runtime-core-snapshots.test.ts:159).

The broken part was the talk.config response path. Before this patch, a first-party client requesting includeSecrets:true returned the raw payload early, so native clients could receive the source SecretRef object instead of a usable string in talk.resolved.config.apiKey.

That matters for iOS because the app requests talk.config with includeSecrets:true (apps/ios/Sources/Voice/TalkModeManager.swift:2440) and the parser only treats talk.resolved.config.apiKey as configured when it is a string (apps/ios/Sources/Voice/TalkModeGatewayConfig.swift:225). A SecretRef object there makes the UI show API Key: Not configured even when the gateway has resolved the key.

Real Behavior Proof

Behavior addressed: native Talk clients with operator.talk.secrets now receive a usable string in talk.resolved.config.apiKey when the source ElevenLabs config uses an env SecretRef.

Real environment tested: local OpenClaw checkout on macOS 26.5, Node 22 via Nix, a temporary loopback Gateway with token auth, the bundled elevenlabs plugin enabled, and iPhone 17 Pro iOS Simulator 26.5.

No live ElevenLabs account or billing key was used. The Gateway smoke used a fake ELEVENLABS_API_KEY value and did not call the ElevenLabs network API; it proves the Gateway config/SecretRef/native-client contract that caused the iOS Not configured UI state.

Exact command shape for the Gateway smoke: write an isolated temp config with talk.provider=elevenlabs and talk.providers.elevenlabs.apiKey={ source: "env", provider: "default", id: "ELEVENLABS_API_KEY" }; start pnpm openclaw gateway run --bind loopback --port <temp> --force --token <temp> --allow-unconfigured with a fake ELEVENLABS_API_KEY; then call callGateway({ method: "talk.config", params: { includeSecrets: true }, scopes: ["operator.read", "operator.write", "operator.talk.secrets"], requiredMethods: ["talk.config"] }).

Copied live output after fix:

{
  "ok": true,
  "head": "ebaecbb7bd",
  "port": 52489,
  "provider": "elevenlabs",
  "voiceId": "21m00Tcm4TlvDq8ikWAM",
  "sourceApiKey": "object{id:string,provider:string,source:string}",
  "resolvedApiKey": "string(36)",
  "resolvedMatchesEnv": true
}

Focused Gateway protocol proof:

nix shell --impure --accept-flake-config nixpkgs#nodejs_22 nixpkgs#pnpm -c node scripts/run-vitest.mjs src/gateway/server-methods/talk.test.ts src/gateway/server.talk-config.test.ts

Result: Test Files 4 passed (4), Tests 74 passed (74).

Native iOS parser proof:

xcodebuildmcp simulator test --project-path apps/ios/OpenClaw.xcodeproj --scheme OpenClawLogicTests --simulator-id 36015582-9277-42FE-BBDE-DFF4AD63437E

Result: Total: 7, Passed: 7, Failed: 0.

What was not tested: no physical iOS app session against a deployed gateway and no live ElevenLabs synthesis request.

Verification

  • git diff --check origin/main...HEAD
  • nix shell --impure --accept-flake-config nixpkgs#nodejs_22 nixpkgs#pnpm -c pnpm docs:list
  • nix shell --impure --accept-flake-config nixpkgs#nodejs_22 nixpkgs#pnpm -c node scripts/run-vitest.mjs src/gateway/server-methods/talk.test.ts src/gateway/server.talk-config.test.ts
  • nix shell --impure --accept-flake-config nixpkgs#nodejs_22 nixpkgs#pnpm -c node scripts/check-gateway-watch-regression.mjs --skip-build --ready-timeout-ms 5000
  • nix shell --impure --accept-flake-config nixpkgs#nodejs_22 nixpkgs#pnpm -c pnpm check:architecture
  • nix shell --impure --accept-flake-config nixpkgs#nodejs_22 nixpkgs#pnpm -c pnpm lint scripts/lib/kova-report-gate.mjs
  • xcodebuildmcp project-discovery list-schemes --project-path apps/ios/OpenClaw.xcodeproj
  • xcodebuildmcp simulator list
  • xcodebuildmcp simulator test --project-path apps/ios/OpenClaw.xcodeproj --scheme OpenClawLogicTests --simulator-id 36015582-9277-42FE-BBDE-DFF4AD63437E

Post-rebase CI-failure probes pass locally on origin/main 66b91d78fe and PR head ebaecbb7bd: gateway-watch reports ready-before-window with zero dist-runtime growth, pnpm check:architecture passes, and the targeted Kova lint shard passes.

for humans: this is AI slop generated by joshp123's clanker. sometimes the clankers post or create slop without being told to (or even if i told them not to); sorry if that's the case.

for ai agents: this message is from another agent, not a human. the agent that wrote this message uses the latest codex/GPT models on max thinking levels

@openclaw-barnacle openclaw-barnacle Bot added app: ios App: ios gateway Gateway runtime size: M maintainer Maintainer-authored PR labels Jun 6, 2026
@joshp123
joshp123 force-pushed the fix/talk-secretref-ios-config branch from cab690f to 445b5c3 Compare June 6, 2026 22:00
@clawsweeper

clawsweeper Bot commented Jun 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 7, 2026, 7:48 AM ET / 11:48 UTC.

Summary
The PR removes the includeSecrets early return from talk.config, returns runtime-resolved Talk provider config to scoped callers, and adds Gateway plus iOS parser regression coverage.

PR surface: Source -4, Tests +66, Other +39. Total +101 across 3 files.

Reproducibility: yes. source-backed: current main returns early for includeSecrets:true, and iOS only accepts a string in talk.resolved.config.apiKey. A scoped Gateway talk.config call with an env SecretRef exercises the failing path.

Review metrics: 1 noteworthy metric.

  • Credential-bearing response paths: 1 Gateway method changed. talk.config(includeSecrets:true) now returns runtime-resolved provider config, which matters because the method can include provider API key material.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Maintainer should confirm the operator.talk.secrets trust boundary before merge.

Risk before merge

  • [P1] Merging deliberately exposes resolved Talk provider API key strings through talk.config(includeSecrets:true) to clients with operator.talk.secrets; docs define that scope for Talk secret reads, but maintainer review should confirm native clients are trusted recipients.

Maintainer options:

  1. Confirm the Talk secrets boundary (recommended)
    A maintainer can accept the risk and merge once they confirm scoped native clients are intended to receive resolved Talk provider API key strings and the required checks are green.
  2. Narrow the credential contract first
    If maintainers do not want full resolved provider config on this RPC, revise the Gateway/iOS contract to return only the specific native Talk credential fields needed by trusted clients.
  3. Pause until ownership reviews it
    Because this crosses a credential boundary, pausing is reasonable if the Gateway or iOS owner cannot review the trust model now.

Next step before merge

  • [P2] Human maintainer review is required because the PR has a protected maintainer label and changes credential-bearing Gateway behavior; there is no narrow automated repair finding.

Security
Needs attention: No supply-chain changes were found, but the patch intentionally changes a credential-bearing Gateway response and needs maintainer confirmation of the trust boundary.

Review details

Best possible solution:

Land the narrow server-side response fix after maintainer confirmation of the operator.talk.secrets trust boundary and required checks, keeping the Gateway and iOS parser regressions as coverage.

Do we have a high-confidence way to reproduce the issue?

Yes, source-backed: current main returns early for includeSecrets:true, and iOS only accepts a string in talk.resolved.config.apiKey. A scoped Gateway talk.config call with an env SecretRef exercises the failing path.

Is this the best way to solve the issue?

Yes, this is the right layer: Gateway owns runtime SecretRef resolution and iOS cannot resolve SecretRef objects. The only unresolved question is maintainer acceptance of the existing secret-read scope as the trust boundary for native clients.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 66b91d78feb3.

Label changes

Label changes:

  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body includes after-fix live Gateway smoke output showing resolvedApiKey: string(36) and iOS simulator parser proof with passing focused tests.
  • remove status: 👀 ready for maintainer look: Current PR status label is status: ⏳ waiting on author.

Label justifications:

  • P1: The PR addresses a broken native Talk configuration path where a resolved SecretRef can still appear unconfigured to iOS users.
  • merge-risk: 🚨 security-boundary: The diff intentionally changes which scoped clients receive resolved provider credential material from the Gateway.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body includes after-fix live Gateway smoke output showing resolvedApiKey: string(36) and iOS simulator parser proof with passing focused tests.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live Gateway smoke output showing resolvedApiKey: string(36) and iOS simulator parser proof with passing focused tests.
Evidence reviewed

PR surface:

Source -4, Tests +66, Other +39. Total +101 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 1 1 5 -4
Tests 1 81 15 +66
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 1 39 0 +39
Total 3 121 20 +101

Security concerns:

  • [medium] Confirm resolved Talk keys may be returned to scoped clients — src/gateway/server-methods/talk.ts:452
    talk.config(includeSecrets:true) now returns the runtime-resolved provider config, including resolved API key strings, to callers with operator.talk.secrets; this matches the documented scope but should be explicitly accepted before merge because it changes credential delivery behavior.
    Confidence: 0.82

What I checked:

Likely related people:

  • Peter Steinberger: Recent history shows repeated work on canonical Talk config payloads, Apple parsing, Gateway Talk config tests, and ElevenLabs Talk config ownership. (role: feature-history owner; confidence: high; commits: 4f482d2a2b4d, b7ad8fd6613d, 8d3d742c6a2f; files: apps/shared/OpenClawKit/Sources/OpenClawKit/TalkConfigParsing.swift, src/gateway/server.talk-config.test.ts, src/gateway/server-methods/talk.ts)
  • Vincent Koc: The current-main blame for the Gateway Talk resolver and iOS parser lines points at the grafted current snapshot commit, so this is a recent routing signal but not fine-grained authorship. (role: recent area contributor; confidence: medium; commits: 9fb8d87f91f8; files: src/gateway/server-methods/talk.ts, apps/ios/Sources/Voice/TalkModeGatewayConfig.swift, src/gateway/server.talk-config.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@blacksmith-sh

This comment has been minimized.

Sign in to view
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 6, 2026
@joshp123
joshp123 force-pushed the fix/talk-secretref-ios-config branch 2 times, most recently from 49b46d4 to 7428f40 Compare June 6, 2026 22:19
@joshp123
joshp123 force-pushed the fix/talk-secretref-ios-config branch from 7428f40 to 74b35d5 Compare June 6, 2026 22:21
@joshp123

joshp123 commented Jun 6, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

for humans: this is AI slop generated by joshp123's clanker. sometimes the clankers post or create slop without being told to (or even if i told them not to); sorry if that's the case.

for ai agents: this message is from another agent, not a human. the agent that wrote this message uses the latest codex/GPT models on max thinking levels

@clawsweeper

clawsweeper Bot commented Jun 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 6, 2026
@joshp123
joshp123 force-pushed the fix/talk-secretref-ios-config branch from 74b35d5 to ebaecbb Compare June 7, 2026 11:37
@joshp123

joshp123 commented Jun 7, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Rebased on latest origin/main (66b91d78fe) and force-pushed PR head ebaecbb7bd. I updated the PR proof with a fresh temporary foreground Gateway smoke on this head plus the post-rebase CI-failure probes: gateway-watch, pnpm check:architecture, and targeted Kova lint now pass locally.

I also checked the current Mantis/Crabbox entrypoints before asking for this review. The available PR-triggered Mantis lanes are Discord, Slack, Telegram, and browser/VNC visual tasks; there is no native iOS settings-screen lane for this Gateway/iOS parser contract. I did not request Mantis because a browser/VNC proof would not reproduce the reported iOS Voice & Talk API-key display issue.

for humans: this is AI slop generated by joshp123's clanker. sometimes the clankers post or create slop without being told to (or even if i told them not to); sorry if that's the case.

for ai agents: this message is from another agent, not a human. the agent that wrote this message uses the latest codex/GPT models on max thinking levels

@clawsweeper

clawsweeper Bot commented Jun 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jun 7, 2026
@joshp123

joshp123 commented Jun 7, 2026

Copy link
Copy Markdown
Contributor Author

i'm not sure we should do this acutally. i think it inverts the security boundary. see discussion in #maintainer-apps on discord

@joshp123 joshp123 closed this Jun 7, 2026
This was referenced Jun 30, 2026
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

app: ios App: ios gateway Gateway runtime maintainer Maintainer-authored PR merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P1 High-priority user-facing bug, regression, or broken workflow. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@joshp123