Skip to content

fix(discord): resolve env-backed SecretRefs in startup token path#76383

Closed
neeravmakwana wants to merge 1 commit intoopenclaw:mainfrom
neeravmakwana:fix/discord-env-secretref-76371
Closed

fix(discord): resolve env-backed SecretRefs in startup token path#76383
neeravmakwana wants to merge 1 commit intoopenclaw:mainfrom
neeravmakwana:fix/discord-env-secretref-76371

Conversation

@neeravmakwana
Copy link
Copy Markdown
Contributor

@neeravmakwana neeravmakwana commented May 3, 2026

Problem

Discord channel startup failed when channels.discord.token used an env-backed SecretRef: normalizeDiscordToken called normalizeResolvedSecretInputString on raw config and threw (env:default:DISCORD_BOT_TOKEN unresolved) before Gateway secrets/runtime resolution ran. Telegram already handled this pattern via inspect + env lookup.

Root cause

resolveDiscordToken normalized account/channel tokens through the strict secret-input helper, which treats unresolved refs as fatal instead of resolving source: "env" refs against secrets.providers, allowlists, and process.env (same chain as Telegram’s bot token resolver).

Fix

  • Resolve Discord account and channel tokens with resolveSecretInputString(..., mode: "inspect"), then follow env SecretRefs through the same provider/allowlist/default-alias rules as Telegram (resolveDefaultSecretProviderAlias, allowlist errors, non-env provider source mismatch throws).
  • Preserve “explicit env ref but empty” semantics: no fallback to bare DISCORD_BOT_TOKEN when a different ref id was configured.

Why this is safe

Behavior is aligned with extensions/telegram/src/token.ts for env-backed refs: same validation surface, same failure modes for misconfigured providers, and strict resolution still applies to non-env refs via the existing mode: "strict" path.

Security / runtime controls (unchanged)

  • Env provider allowlists still enforced.
  • Unconfigured/non-env-backed providers still error as before when an env ref names them.
  • Non-env unresolved SecretRefs still resolve through strict mode (throw), not silently dropped.

Tests run

  • pnpm exec vitest run extensions/discord/src/token.test.ts
  • pnpm test:extension discord
  • pnpm check:changed

Follow-ups (out of scope)

Fixes #76371

  • AI-assisted (implementation and validation)
  • Testing: pnpm test:extension discord + targeted token tests + pnpm check:changed

@neeravmakwana @cursoragent
fix(discord): resolve env SecretRefs during token resolution
bcfec22 
Align resolveDiscordToken with Telegram: inspect SecretRefs, resolve env:provider:id via secrets.providers/allowlists, keep strict unresolved non-env refs.

Fixes openclaw#76371.

Co-authored-by: Cursor <cursoragent@cursor.com>
@neeravmakwana neeravmakwana mentioned this pull request May 3, 2026
Closed
@openclaw-barnacle openclaw-barnacle Bot added channel: discord Channel integration: discord size: S labels May 3, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 3, 2026 •
edited
Loading

Codex review: needs maintainer review before merge.

Summary
The PR updates Discord token resolution, regression tests, and changelog so env-backed channels.discord SecretRefs resolve through env provider/default/allowlist rules during startup instead of strict unresolved-ref rejection.

Reproducibility: yes. Source inspection of current main shows resolveDiscordToken still uses the strict SecretRef normalizer, and the current Discord token test explicitly expects the env SecretRef case to throw.

Next step before merge
No repair job is needed because this PR already contains the focused implementation, tests, and changelog entry; the remaining action is normal maintainer review and CI.

Security
Cleared: The diff preserves env provider source/default/allowlist checks and does not add dependencies, workflows, publishing metadata, or secret logging.

Review details

Best possible solution:

Land the focused Discord plugin fix after normal CI, while keeping broader runtime-snapshot and non-env SecretRef work tracked separately.

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

Yes. Source inspection of current main shows resolveDiscordToken still uses the strict SecretRef normalizer, and the current Discord token test explicitly expects the env SecretRef case to throw.

Is this the best way to solve the issue?

Yes. The proposed change is a narrow Discord-owned repair aligned with Telegram's env SecretRef behavior and preserves strict handling for non-env refs, leaving the broader runtime-snapshot problem to separate issues.

What I checked:

Likely related people:

  • Vincent Koc: Git history and blame show commit dd43caa introduced the current Discord token resolver, account resolver, setup account state, SecretRef contract entries, and Discord SecretRef docs now involved in this path. (role: introduced behavior; confidence: high; commits: dd43caa27aa1; files: extensions/discord/src/token.ts, extensions/discord/src/accounts.ts, extensions/discord/src/setup-account-state.ts)
  • Peter Steinberger: Commit 8b2a6e5 refreshed the bundled channel/plugin inventory for the latest release and touched the exact Discord token, test, and SecretRef docs files that define the shipped surface. (role: recent release maintainer; confidence: medium; commits: 8b2a6e57fef6; files: extensions/discord/src/token.ts, extensions/discord/src/token.test.ts, docs/channels/discord.md)

Remaining risk / open question:

  • I did not run tests in this read-only review, so CI should confirm the PR author's listed pnpm test:extension discord and changed-gate validation.
  • Non-env/raw-config SecretRef crashes remain outside this PR and should stay tracked by the related SecretRef follow-ups.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 31161abd40fd.

@clawsweeper clawsweeper Bot mentioned this pull request May 3, 2026
Closed
29 tasks
@mogglemoss
Copy link
Copy Markdown
Contributor

mogglemoss commented May 3, 2026

Hi @neeravmakwana — overlap heads-up: I opened #76385 4 minutes after this one for the same bug (originally filed as #76371 from a real 2026.5.2 install hitting it in production). Caught the duplication via the Codex review on mine, which now explicitly points at this PR as the broader / more canonical fix.

Your Telegram-style approach (resolve env SecretRefs by their own id after provider policy check, applied to both top-level and account tokens) covers the docs-supported DISCORD_PERSONAL_TOKEN / DISCORD_WORK_TOKEN multi-account env-SecretRef pattern. Mine narrowly matches only the DISCORD_BOT_TOKEN env-fallback case and returns source=none for the rest — which Codex correctly flagged as a P2 gap on my PR.

Closing #76385 in favor of this one and adding a Refs #76371 cross-link there. Happy to test against my live 2026.5.2 repro environment if useful — the workaround on my box is currently del(.channels.discord.token) from openclaw.json so the env fallback fires unconditionally; with your patch installed the SecretRef should resolve cleanly. Ping me if you want a smoke test before maintainer review.

@joshavant joshavant mentioned this pull request May 3, 2026
Merged
25 tasks
@joshavant
Copy link
Copy Markdown
Contributor

joshavant commented May 3, 2026

Thanks @neeravmakwana for the patch. The merged fix in #76449 landed the broader Discord solution by resolving external channel secret contracts generically, so the startup token path is covered without a Discord-only workaround. Closing this PR as superseded by the merged fix.

@joshavant joshavant closed this May 3, 2026
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

channel: discord Channel integration: discord size: S

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug] @openclaw/discord 2026.5.2 channel startup crashes on SecretRef-backed token (env:default:DISCORD_BOT_TOKEN unresolved)

3 participants

@neeravmakwana @mogglemoss @joshavant