Repository navigation
fix(discord-voice): default DISCORD_VOICE_OWNER to false (safe) - #973
Merged
Merged
Conversation
Anyone who can speak in the voice channel previously inherited owner-tier work/file-edit/message-send privileges by default. That's fine for a single-operator Lounge, but it's a sharp edge to ship as the default — inviting the bot to any channel with other speakers silently hands them the proactive-loop surface. Flip the default to false: non-owner speakers get the read-only tools only. To restore the single-operator behavior, set DISCORD_VOICE_OWNER=true explicitly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
qingyun-wu
approved these changes
May 21, 2026
qingyun-wu
left a comment
qingyun-wu
left a comment
Collaborator
There was a problem hiding this comment.
[core: legacy]
Approve — security-positive default flip on a personal-AI install. Clean 2-file change.
One subtle behavior change worth flagging in the PR body:
The parsing tightened beyond just the default flip:
- Before:
(process.env.DISCORD_VOICE_OWNER ?? 'true') !== 'false'→ owner mode on for any non-literal-falsevalue (1,yes,TRUE, etc.). - After:
(process.env.DISCORD_VOICE_OWNER ?? 'false') === 'true'→ owner mode only for the literal stringtrue.
Net: existing operators who set DISCORD_VOICE_OWNER=1 or DISCORD_VOICE_OWNER=TRUE thinking they had owner-mode pinned will now silently fall through to the safe surface. For a security-positive flip this is the right call (fail-closed beats fail-open) — but the PR body's "set DISCORD_VOICE_OWNER=true explicitly to restore" doesn't quite cover the case. A one-line note in ## Why would prevent surprise.
Optional follow-up (non-blocking, not for this PR):
A startup log line — [discord-voice] owner-tier inheritance: <on|off> — would let operators verify the env survived their install/restart without having to speak in the channel and try a privileged tool. Cheap, surfaces the trust state.
Otherwise SKILL.md restructuring is clear; trust-boundary section now reads as the right shape for the safer default. Test plan covers the three real scenarios.
liususan091219
approved these changes
May 21, 2026
liususan091219
left a comment
liususan091219
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed post-merge per request. The default flip DISCORD_VOICE_OWNER true→false is fail-safe — non-owner speakers no longer auto-inherit owner-tier work/file-edit/message-send. The parsing tightening (?? 'true') !== 'false' → (?? 'false') === 'true' is also correct: the old form treated any non-'false' value (incl. 'False', '0', empty string) as owner; the new form requires an exact 'true'. Approving. Non-blocking doc nit (already flagged by Maddy): SKILL.md describes a non-owner 'read-only tool surface' but the non-owner branch builds an empty tools array — worth reconciling in a follow-up.
3 tasks done
liususan091219
added a commit
that referenced
this pull request
May 21, 2026
Owner-tier tools were gated by a process-global DISCORD_VOICE_OWNER flag — all-or-nothing for the whole channel. In a shared voice channel that means either every speaker (incl. strangers) gets owner-tier `work` / file-edit / message-send, or nobody does. No middle ground, no per-speaker security (flagged in the #973 review). Add per-speaker tiering, mirroring the discord-bridge model and read from the same `~/.claude/channels/discord/access.json`: - owner — the `owner` field (instance operator) → full tool surface - team — the rest of `allowFrom` → inline + configurable tools - other — anyone else → read-only inline tools Each turn is attributed to its speaker (`speaking.start` → lastSpeakerId); owner/team tools are wrapped so the tier is enforced at execute() time (the Gemini session's tool list is fixed at session start). DISCORD_VOICE_OWNER remains as the fallback when no `owner` is configured. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sonichi
added a commit
that referenced
this pull request
May 22, 2026
* feat(discord-voice): per-speaker access tiers (owner/team/other) Owner-tier tools were gated by a process-global DISCORD_VOICE_OWNER flag — all-or-nothing for the whole channel. In a shared voice channel that means either every speaker (incl. strangers) gets owner-tier `work` / file-edit / message-send, or nobody does. No middle ground, no per-speaker security (flagged in the #973 review). Add per-speaker tiering, mirroring the discord-bridge model and read from the same `~/.claude/channels/discord/access.json`: - owner — the `owner` field (instance operator) → full tool surface - team — the rest of `allowFrom` → inline + configurable tools - other — anyone else → read-only inline tools Each turn is attributed to its speaker (`speaking.start` → lastSpeakerId); owner/team tools are wrapped so the tier is enforced at execute() time (the Gemini session's tool list is fixed at session start). DISCORD_VOICE_OWNER remains as the fallback when no `owner` is configured. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(discord-voice): extract tier logic to access-tier.ts + unit tests Split the per-speaker tier logic out of discord-voice-server.ts into a pure, importable access-tier.ts (tierFor / toolAllowed / loadAccessTiers) so it can be unit-tested without a live voice session. Adds tests/discord-voice-tier.test.ts — 8 cases covering owner/team/other resolution (incl. unknown id, undefined speaker, no-owner-configured) and the owner-only / team / open tool-gate decisions. discord-voice-server.ts now imports the module instead of inlining the logic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(discord-voice): owner-gate the DM-triggered voice-channel join A 'join voice' request is now explicitly owner-only: honored only when the originating task's access_tier is owner. team/other requests to make the bot enter a voice channel are declined. Documents the two-layer protection — non-owner Discord tasks already route to a read-only sandbox that can't spawn the server, and the join request is owner-gated on top. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(discord-voice): team tier may dismiss the voice session Per the access-tier policy settled by Chi: team-tier speakers get `dismiss` (end the voice session) in addition to the read-only + configurable surface — a teammate in a shared channel can send the bot home when the owner isn't present to close the room. `work` stays owner-only. Moves `dismiss` from OWNER_ONLY to OWNER_OR_TEAM in the per-speaker gate; SKILL.md team bullet updated to flag the UX consequence. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(discord-voice): cover per-tool tiering — dismiss=team, work=owner Extends the tier mock test to assert the per-tool classification, not just the tier-decision function. The f15df99 policy change (dismiss is team-tier) is now encoded in a testable SKILL_TOOL_TIER map + toolNeed() in access-tier.ts; the gate loop uses toolNeed() instead of inline Sets. discord-voice-tier.test.ts → 13/13: adds SKILL_TOOL_TIER (dismiss=team, work/screen-share=owner) and toolNeed (registry + open-tool) cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(discord-voice): per-turn speaker attribution + legacy-owner fallback Addresses two findings from Mini's #984 review. 1. TOCTOU in the tier gate: it read `s.lastSpeakerId` — the live global last speaker — at tool execute() time. Between Gemini deciding to call a tool and execute() running, another speaker could flip lastSpeakerId, so a non-owner's owner-tier request could execute as owner (or a valid owner request be denied). Replaced with a per-turn speaker set (`turnSpeakers`): every speaking.start id is added, cleared on turn.end. `effectiveTier()` resolves the turn across the whole set, failing closed to the least-privileged speaker — never escalating. 2. Legacy DISCORD_VOICE_OWNER hatch was broken: with TREAT_AS_OWNER set but no `owner` in access.json, owner tools were declared but the gate still denied them. `effectiveTier()` now returns `owner` when treatAsOwner. Tier logic stays pure/unit-tested in access-tier.ts; +9 tests (mostRestrictiveTier, effectiveTier incl. the TOCTOU and legacy cases). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(discord-voice): align access-tier model with discord-bridge (finding 3) Per Chi's call on the #984 finding-3 review: access.json's top-level `allowFrom` is canonically the OWNER tier — matching discord-bridge.py. access-tier.ts previously read the `owner` field as owner and top-level `allowFrom` as team, which contradicted discord-bridge.py (it reads top-level `allowFrom` as owner, groups[*].allowFrom as team). That contradiction meant adding a team voice user to `allowFrom` could silently grant them owner-tier text/DM tasks via the bridge. - loadAccessTiers: owner = top-level `allowFrom`; team = union of `groups[*].allowFrom`. AccessTiers.owner is now a Set<string>. - tierFor: owner checked first (precedence — an id in both resolves owner). - discord-voice-server.ts: isOwner now uses ACCESS.owner.size > 0 (the type changed string -> Set). - SKILL.md trust-boundary section updated to the aligned model. - tier unit test 22 -> 25: owner-precedence + loadAccessTiers parsing (confirms top-level allowFrom -> owner, groups -> team, and that the `owner` field is no longer a tier source). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Chi Wang <4250911+sonichi@users.noreply.github.com>
2 of 3 tasks
liususan091219
pushed a commit
to liususan091219/sutando-fork
that referenced
this pull request
May 28, 2026
…chi#973) Anyone who can speak in the voice channel previously inherited owner-tier work/file-edit/message-send privileges by default. That's fine for a single-operator Lounge, but it's a sharp edge to ship as the default — inviting the bot to any channel with other speakers silently hands them the proactive-loop surface. Flip the default to false: non-owner speakers get the read-only tools only. To restore the single-operator behavior, set DISCORD_VOICE_OWNER=true explicitly. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Qingyun Wu <qingyun0327@gmail.com>
liususan091219
added a commit
to liususan091219/sutando-fork
that referenced
this pull request
May 28, 2026
…ichi#984) * feat(discord-voice): per-speaker access tiers (owner/team/other) Owner-tier tools were gated by a process-global DISCORD_VOICE_OWNER flag — all-or-nothing for the whole channel. In a shared voice channel that means either every speaker (incl. strangers) gets owner-tier `work` / file-edit / message-send, or nobody does. No middle ground, no per-speaker security (flagged in the sonichi#973 review). Add per-speaker tiering, mirroring the discord-bridge model and read from the same `~/.claude/channels/discord/access.json`: - owner — the `owner` field (instance operator) → full tool surface - team — the rest of `allowFrom` → inline + configurable tools - other — anyone else → read-only inline tools Each turn is attributed to its speaker (`speaking.start` → lastSpeakerId); owner/team tools are wrapped so the tier is enforced at execute() time (the Gemini session's tool list is fixed at session start). DISCORD_VOICE_OWNER remains as the fallback when no `owner` is configured. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(discord-voice): extract tier logic to access-tier.ts + unit tests Split the per-speaker tier logic out of discord-voice-server.ts into a pure, importable access-tier.ts (tierFor / toolAllowed / loadAccessTiers) so it can be unit-tested without a live voice session. Adds tests/discord-voice-tier.test.ts — 8 cases covering owner/team/other resolution (incl. unknown id, undefined speaker, no-owner-configured) and the owner-only / team / open tool-gate decisions. discord-voice-server.ts now imports the module instead of inlining the logic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(discord-voice): owner-gate the DM-triggered voice-channel join A 'join voice' request is now explicitly owner-only: honored only when the originating task's access_tier is owner. team/other requests to make the bot enter a voice channel are declined. Documents the two-layer protection — non-owner Discord tasks already route to a read-only sandbox that can't spawn the server, and the join request is owner-gated on top. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(discord-voice): team tier may dismiss the voice session Per the access-tier policy settled by Chi: team-tier speakers get `dismiss` (end the voice session) in addition to the read-only + configurable surface — a teammate in a shared channel can send the bot home when the owner isn't present to close the room. `work` stays owner-only. Moves `dismiss` from OWNER_ONLY to OWNER_OR_TEAM in the per-speaker gate; SKILL.md team bullet updated to flag the UX consequence. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(discord-voice): cover per-tool tiering — dismiss=team, work=owner Extends the tier mock test to assert the per-tool classification, not just the tier-decision function. The f15df99 policy change (dismiss is team-tier) is now encoded in a testable SKILL_TOOL_TIER map + toolNeed() in access-tier.ts; the gate loop uses toolNeed() instead of inline Sets. discord-voice-tier.test.ts → 13/13: adds SKILL_TOOL_TIER (dismiss=team, work/screen-share=owner) and toolNeed (registry + open-tool) cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(discord-voice): per-turn speaker attribution + legacy-owner fallback Addresses two findings from Mini's sonichi#984 review. 1. TOCTOU in the tier gate: it read `s.lastSpeakerId` — the live global last speaker — at tool execute() time. Between Gemini deciding to call a tool and execute() running, another speaker could flip lastSpeakerId, so a non-owner's owner-tier request could execute as owner (or a valid owner request be denied). Replaced with a per-turn speaker set (`turnSpeakers`): every speaking.start id is added, cleared on turn.end. `effectiveTier()` resolves the turn across the whole set, failing closed to the least-privileged speaker — never escalating. 2. Legacy DISCORD_VOICE_OWNER hatch was broken: with TREAT_AS_OWNER set but no `owner` in access.json, owner tools were declared but the gate still denied them. `effectiveTier()` now returns `owner` when treatAsOwner. Tier logic stays pure/unit-tested in access-tier.ts; +9 tests (mostRestrictiveTier, effectiveTier incl. the TOCTOU and legacy cases). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(discord-voice): align access-tier model with discord-bridge (finding 3) Per Chi's call on the sonichi#984 finding-3 review: access.json's top-level `allowFrom` is canonically the OWNER tier — matching discord-bridge.py. access-tier.ts previously read the `owner` field as owner and top-level `allowFrom` as team, which contradicted discord-bridge.py (it reads top-level `allowFrom` as owner, groups[*].allowFrom as team). That contradiction meant adding a team voice user to `allowFrom` could silently grant them owner-tier text/DM tasks via the bridge. - loadAccessTiers: owner = top-level `allowFrom`; team = union of `groups[*].allowFrom`. AccessTiers.owner is now a Set<string>. - tierFor: owner checked first (precedence — an id in both resolves owner). - discord-voice-server.ts: isOwner now uses ACCESS.owner.size > 0 (the type changed string -> Set). - SKILL.md trust-boundary section updated to the aligned model. - tier unit test 22 -> 25: owner-precedence + loadAccessTiers parsing (confirms top-level allowFrom -> owner, groups -> team, and that the `owner` field is no longer a tier source). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Chi Wang <4250911+sonichi@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DISCORD_VOICE_OWNERdefault fromtrue→falseinskills/discord-voice/scripts/discord-voice-server.ts.skills/discord-voice/SKILL.mdso docs match the new safe default.Why
The old default meant any speaker in the voice channel inherited owner-tier
work/ file-edit / message-send privileges. Fine for a single-operator Lounge — sharp edge to ship as the default. Anyone joining a channel the bot was in could delegateworkor edit files just by speaking.Now non-owner speakers get the read-only tool surface (lookups, status checks); only the configured
ownerenv gets the owner-tier surface. To restore the previous behavior, setDISCORD_VOICE_OWNER=trueexplicitly (single-operator personal-use mode).Test plan
npx tsc --noEmitcleanownermatches caller id → unchanged)DISCORD_VOICE_OWNER=trueexplicitly: every speaker gets owner surface (legacy behavior)🤖 Generated with Claude Code