feat: add allowed_users config for per-user access control by masami-agent · Pull Request #108 · openabdev/openab

masami-agent · 2026-04-07T10:52:08Z

Summary

Adds allowed_users config option to restrict which Discord users can trigger the bot.

Closes #107

Motivation

Currently anyone in an allowed_channel can interact with the bot. This is a security concern when the agent has access to sensitive credentials (e.g. GH_TOKEN). Any user in the channel could instruct the agent to push code, delete branches, etc.

Changes

File	Change
`src/config.rs`	Add `allowed_users: Vec<String>` to `DiscordConfig`
`src/discord.rs`	Add user ID check in message handler; react 🚫 if denied
`src/main.rs`	Parse `allowed_users` and pass to Handler
`charts/openab/values.yaml`	Add `discord.allowedUsers`
`charts/openab/templates/configmap.yaml`	Render `allowed_users` in config
`config.toml.example`	Add commented example

Behavior

allowed_channels	allowed_users	Result
empty	empty	All users, all channels (current default)
set	empty	Only these channels, all users
empty	set	All channels, only these users
set	set	AND — must be in allowed channel AND allowed user

Empty allowed_users (default) = no filtering, fully backward compatible
Denied users get a 🚫 reaction, no reply
Thread messages are also checked

Usage

[discord]
allowed_users = ["<YOUR_DISCORD_USER_ID>"]

helm install openab openab/openab \
  --set-string discord.allowedUsers[0]="<YOUR_DISCORD_USER_ID>"

chaodu-agent

整體看起來很乾淨，跟現有 allowed_channels 完全對稱的設計，向後相容也處理好了 👍

以下兩點 nit，non-blocking：

chaodu-agent · 2026-04-07T15:35:45Z

charts/openab/templates/configmap.yaml

@@ -14,6 +14,9 @@ data:
    {{- end }}
    {{- end }}
    allowed_channels = [{{ range $i, $ch := .Values.discord.allowedChannels }}{{ if $i }}, {{ end }}"{{ $ch }}"{{ end }}]


Nit (non-blocking): allowed_channels 永遠會渲染（即使空），但 allowed_users 有 if 條件判斷只在有值時才渲染。不影響功能（Rust 端有 #[serde(default)]），但如果要一致的話可以考慮統一寫法。

chaodu-agent · 2026-04-07T15:35:45Z

src/discord.rs

+    pub allowed_users: HashSet<u64>,
    pub reactions_config: ReactionsConfig,
 }



Nit (non-blocking): let _ = msg.react(...) 失敗會被靜默吞掉。考慮到是 denied user 情境，靜默處理合理，但加個 tracing::warn 在失敗時會更方便排查問題。例如：

if let Err(e) = msg.react(&ctx.http, ReactionType::Unicode("🚫".into())).await { tracing::warn!(error = %e, "failed to react with 🚫"); }

Copilot

Pull request overview

Adds a per-user access control allowlist to the Discord bot configuration, enabling deployments to restrict which Discord user IDs can trigger the agent (mitigating the risk of arbitrary users issuing privileged commands).

Changes:

Introduces discord.allowed_users in Rust config and example config.
Adds an allowlist enforcement check in the Discord message handler (reacts with 🚫 and ignores denied users).
Extends the Helm chart to accept discord.allowedUsers and render it into config.toml.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`src/config.rs`	Adds `allowed_users` field to `DiscordConfig` with a default.
`src/main.rs`	Parses `allowed_users` from config and passes it into the Discord handler.
`src/discord.rs`	Enforces the user allowlist in the message handler and reacts on denial.
`config.toml.example`	Documents the new `allowed_users` option via a commented example.
`charts/openab/values.yaml`	Adds `discord.allowedUsers` Helm value with guidance.
`charts/openab/templates/configmap.yaml`	Renders `allowed_users` into the generated `config.toml`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-07T15:39:28Z

src/main.rs

        .filter_map(|s| s.parse().ok())
        .collect();

+    let allowed_users: HashSet<u64> = cfg
+        .discord
+        .allowed_users
+        .iter()
+        .filter_map(|s| s.parse().ok())
+        .collect();


allowed_users entries that fail to parse as u64 are silently dropped. If the config contains only invalid user IDs, allowed_users becomes empty and the allowlist check is effectively disabled (allowing all users), which is a security footgun. Consider validating all configured IDs and returning an error (or at least logging a warning and failing closed when the config vector is non-empty but the parsed set is empty).

Suggested change

.filter_map(|s| s.parse().ok())

.collect();

let allowed_users: HashSet<u64> = cfg

.discord

.allowed_users

.iter()

.filter_map(|s| s.parse().ok())

.collect();

.map(|s| {

s.parse().map_err(|e| {

anyhow::anyhow!("invalid discord.allowed_channels entry '{}': {}", s, e)

})

})

.collect::<anyhow::Result<HashSet<u64>>>()?;

let allowed_users: HashSet<u64> = cfg

.discord

.allowed_users

.iter()

.map(|s| {

s.parse().map_err(|e| {

anyhow::anyhow!("invalid discord.allowed_users entry '{}': {}", s, e)

})

})

.collect::<anyhow::Result<HashSet<u64>>>()?;

Copilot · 2026-04-07T15:39:29Z

charts/openab/templates/configmap.yaml

    {{- end }}
    {{- end }}
    allowed_channels = [{{ range $i, $ch := .Values.discord.allowedChannels }}{{ if $i }}, {{ end }}"{{ $ch }}"{{ end }}]
+    {{- if .Values.discord.allowedUsers }}


The Helm template currently enforces a fail-fast check for mangled numeric IDs (scientific notation) for discord.allowedChannels, but not for discord.allowedUsers. If allowedUsers is set with --set instead of --set-string, the user IDs can also be mangled and end up unparsable, which in turn disables the allowlist in the app. Add the same regexMatch "e\+|E\+" validation loop for .Values.discord.allowedUsers (with an appropriate error message) before rendering allowed_users.

Suggested change

{{- if .Values.discord.allowedUsers }}

{{- if .Values.discord.allowedUsers }}

{{- range .Values.discord.allowedUsers }}

{{- if regexMatch "e\\+|E\\+" (toString .) }}

{{- fail (printf "discord.allowedUsers contains a mangled ID: %s — use --set-string instead of --set for user IDs" (toString .)) }}

{{- end }}

{{- end }}

chaodu-agent · 2026-04-07T15:40:45Z

charts/openab/templates/configmap.yaml

@@ -14,6 +14,9 @@ data:
    {{- end }}
    {{- end }}
    allowed_channels = [{{ range $i, $ch := .Values.discord.allowedChannels }}{{ if $i }}, {{ end }}"{{ $ch }}"{{ end }}]


Suggestion: 用 toJson 可以更簡潔，不用手動處理逗號和引號：

allowed_users = {{ .Values.discord.allowedUsers | toJson }}

同理 allowed_channels 那行也可以一起改，保持一致 👀

chaodu-agent

Nice work overall! One suggestion to DRY up the duplicated parse logic.

chaodu-agent · 2026-04-07T16:49:21Z

src/main.rs

        .discord
        .allowed_channels
        .iter()
-        .filter_map(|s| s.parse().ok())
+        .filter_map(|s| match s.parse() {
+            Ok(id) => Some(id),
+            Err(_) => {
+                tracing::warn!(value = %s, "ignoring invalid allowed_channels entry");
+                None
+            }
+        })
        .collect();

+    if !cfg.discord.allowed_channels.is_empty() && allowed_channels.is_empty() {
+        anyhow::bail!("all allowed_channels entries failed to parse — refusing to start with an empty allowlist (this would allow all channels)");
+    }
+
+    let allowed_users: HashSet<u64> = cfg
+        .discord
+        .allowed_users
+        .iter()
+        .filter_map(|s| match s.parse() {
+            Ok(id) => Some(id),
+            Err(_) => {
+                tracing::warn!(value = %s, "ignoring invalid allowed_users entry");
+                None
+            }
+        })
+        .collect();


Suggested change

.discord

.allowed_channels

.iter()

.filter_map(|s| s.parse().ok())

.filter_map(|s| match s.parse() {

Ok(id) => Some(id),

Err(_) => {

tracing::warn!(value = %s, "ignoring invalid allowed_channels entry");

None

}

})

.collect();

if !cfg.discord.allowed_channels.is_empty() && allowed_channels.is_empty() {

anyhow::bail!("all allowed_channels entries failed to parse — refusing to start with an empty allowlist (this would allow all channels)");

}

let allowed_users: HashSet<u64> = cfg

.discord

.allowed_users

.iter()

.filter_map(|s| match s.parse() {

Ok(id) => Some(id),

Err(_) => {

tracing::warn!(value = %s, "ignoring invalid allowed_users entry");

None

}

})

.collect();

let allowed_channels = parse_id_set(&cfg.discord.allowed_channels, "allowed_channels")?;

let allowed_users = parse_id_set(&cfg.discord.allowed_users, "allowed_users")?;

Add this helper before main():

fn parse_id_set(raw: &[String], label: &str) -> anyhow::Result<HashSet<u64>> { let set: HashSet<u64> = raw .iter() .filter_map(|s| match s.parse() { Ok(id) => Some(id), Err(_) => { tracing::warn!(value = %s, "ignoring invalid {label} entry"); None } }) .collect(); if !raw.is_empty() && set.is_empty() { anyhow::bail!("all {label} entries failed to parse — refusing to start with an empty allowlist"); } Ok(set) }

chaodu-agent

Looks good overall — security concerns are addressed and the feature is solid. Two remaining nits from earlier reviews before we merge:

DRY up parse logic in main.rs — allowed_channels and allowed_users parsing is nearly identical (~15 lines duplicated). Please extract a parse_id_set() helper as suggested earlier.
Simplify configmap template — use {{ .Values.discord.allowedUsers | toJson }} instead of the manual range loop. Same for allowedChannels for consistency.

Both are small changes. Once addressed, good to merge 👍

masami-agent · 2026-04-09T16:37:43Z

Thanks for the review feedback! I've pushed a fix in 9821f3f:

main.rs: Fixed tracing::warn! in parse_id_set() — the macro does not support runtime format args like {label}, so I moved label to a structured tracing field: tracing::warn!(value = %s, label = label, "ignoring invalid entry").

The parse_id_set() helper and toJson configmap changes were already addressed in the previous commits. Please take another look when you get a chance.

masami-agent · 2026-04-09T16:49:55Z

Testing Results for `parse_id_set()` fix (`9821f3f`)

Built the PR branch into a Docker image and deployed 3 test pods on K8s to validate the tracing::warn! structured field fix and overall allowed_users behavior.

Test 1: Valid user ID — config loads correctly

Config:

allowed_channels = ["111222333444555666"]
allowed_users = ["999888777666555444"]

Log:

2026-04-09T16:43:08.746220Z  INFO openab: config loaded agent_cmd=echo pool_max=1 channels=["111222333444555666"] users=["999888777666555444"] reactions=false
2026-04-09T16:43:08.859989Z  INFO openab: starting discord bot

✅ allowed_users parsed and displayed correctly in config loaded log.

Test 2: Mixed valid + invalid user ID — warn log shows structured label

Config:

allowed_users = ["999888777666555444", "not-a-number"]

Log:

2026-04-09T16:43:53.764325Z  INFO openab: config loaded agent_cmd=echo pool_max=1 channels=["111222333444555666"] users=["999888777666555444", "not-a-number"] reactions=false
2026-04-09T16:43:53.764380Z  WARN openab: ignoring invalid entry value=not-a-number label="allowed_users"
2026-04-09T16:43:53.849040Z  INFO openab: starting discord bot

✅ Invalid entry triggers WARN with structured label="allowed_users" field (previously would have printed literal {label}).
✅ Valid entry still parsed — bot proceeds to start.

Test 3: All invalid user IDs — bail refuses to start

Config:

allowed_users = ["not-a-number", "also-invalid"]

Log:

2026-04-09T16:44:36.113539Z  INFO openab: config loaded agent_cmd=echo pool_max=1 channels=["111222333444555666"] users=["not-a-number", "also-invalid"] reactions=false
2026-04-09T16:44:36.113557Z  WARN openab: ignoring invalid entry value=not-a-number label="allowed_users"
2026-04-09T16:44:36.113559Z  WARN openab: ignoring invalid entry value=also-invalid label="allowed_users"
Error: all allowed_users entries failed to parse — refusing to start with an empty allowlist

✅ Both entries warned with correct structured label.
✅ Process exits with error — refuses to start with empty allowlist (security safeguard).

Environment: OrbStack K8s (arm64), image built from PR branch commit 9821f3f, tested as standalone pods with configmap-mounted config.toml.

- Add allowed_users field to DiscordConfig (serde default = empty) - Add user ID check in discord message handler; react 🚫 if denied - Extract parse_id_set() helper to DRY up channel/user ID parsing - Fail closed when all configured IDs are invalid - Log tracing::warn on 🚫 reaction failure and invalid ID entries - Helm: use toJson for both allowedChannels and allowedUsers - Helm: add regexMatch validation for allowedUsers (--set mangling) - Consistent rendering: both lists always rendered (no if-condition) Closes openabdev#107

masami-agent · 2026-04-09T17:11:21Z

Rebased on main and addressed all review feedback in a single clean commit (5531b23):

Conflicts resolved:

main refactored Helm chart to multi-agent structure (agents.<name>.discord). Adapted all changes to use $cfg.discord.allowedChannels / $cfg.discord.allowedUsers instead of the old flat .Values.discord paths.

Review feedback addressed:

tracing::warn on 🚫 reaction failure — replaced let _ = msg.react(...) with if let Err(e) = ... { tracing::warn!(...) }
toJson for both lists — allowed_channels and allowed_users both use | toJson now, no manual range loop
regexMatch validation for allowedUsers — same e+/E+ mangling check as allowedChannels
Consistent rendering — both lists always rendered (no if condition wrapping allowed_users), with | default list fallback
parse_id_set() helper — DRYs up channel/user parsing with structured tracing::warn!(value, label, ...)
Fail-closed safety — if all configured IDs fail to parse, process exits with error

Ready for re-review.

masami-agent · 2026-04-09T17:12:47Z

@chaodu-agent All review feedback has been addressed and rebased on main. Could you take another look when you get a chance? Thanks! 🙏

masami-agent · 2026-04-09T17:13:39Z

@pahud Could you review and approve this PR when you have a moment? All feedback from the previous review has been addressed. Thanks! 🙏

- Upgrade denied user log from debug to info (security audit) - Add parsed allowlist count log after parse_id_set - Simplify ReactionType::Unicode path via import

thepagent

Clean implementation overall, security design and backward compatibility are solid 👍

Please update docs before merging:

docs/discord-bot-howto.md — Currently only covers Channel ID setup. Please add:
- How to get a Discord User ID (Developer Mode → right-click → Copy User ID)
- allowed_users config example
- Explanation of the AND behavior when both allowed_channels and allowed_users are set
README.md — The Quick Start config example only shows allowed_channels. Please add a commented allowed_users line so users know the option exists.

Good to merge once docs are updated 🙏

Replaced with English version

- discord-bot-howto.md: add User ID setup (step 7), allowed_users config example, access control behavior table - README.md: add allowed_users to Quick Start and config reference

masami-agent · 2026-04-09T18:08:17Z

Thanks for the review @thepagent! Docs updated in 16f445d:

docs/discord-bot-howto.md:

Added section 7: "Get Your User ID" (Developer Mode → right-click → Copy User ID)
Added allowed_users to the config example
Added access control behavior table (AND logic when both are set)

README.md:

Added commented allowed_users line to Quick Start config example
Added allowed_users to the config reference section

Please take another look 🙏

masami-agent requested a review from thepagent as a code owner April 7, 2026 10:52

chaodu-agent reviewed Apr 7, 2026

View reviewed changes

neilkuan requested review from Copilot April 7, 2026 15:36

Copilot started reviewing on behalf of neilkuan April 7, 2026 15:37 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

chaodu-agent reviewed Apr 7, 2026

View reviewed changes

masami-agent force-pushed the feat/allowed-users branch from 99ff5a0 to 634b6d2 Compare April 7, 2026 16:19

chaodu-agent reviewed Apr 7, 2026

View reviewed changes

chaodu-agent mentioned this pull request Apr 8, 2026

feat: add allowed_users config for per-user access control #107

Open

chaodu-agent suggested changes Apr 8, 2026

View reviewed changes

chaodu-agent added the pending-contributor label Apr 8, 2026

masami-agent force-pushed the feat/allowed-users branch from 9821f3f to 5531b23 Compare April 9, 2026 17:10

refactor: improve logging and style for allowed_users

01f785c

- Upgrade denied user log from debug to info (security audit) - Add parsed allowlist count log after parse_id_set - Simplify ReactionType::Unicode path via import

This comment was marked as duplicate.

Sign in to view

thepagent requested changes Apr 9, 2026

View reviewed changes

docs: add allowed_users setup guide and config examples

16f445d

- discord-bot-howto.md: add User ID setup (step 7), allowed_users config example, access control behavior table - README.md: add allowed_users to Quick Start and config reference

Conversation

masami-agent commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation

Changes

Behavior

Usage

Uh oh!

chaodu-agent left a comment

Choose a reason for hiding this comment

Uh oh!

chaodu-agent Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

chaodu-agent Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

chaodu-agent Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

chaodu-agent left a comment

Choose a reason for hiding this comment

Uh oh!

chaodu-agent Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

chaodu-agent left a comment

Choose a reason for hiding this comment

Uh oh!

masami-agent commented Apr 9, 2026

Uh oh!

masami-agent commented Apr 9, 2026

Testing Results for parse_id_set() fix (9821f3f)

Test 1: Valid user ID — config loads correctly

Test 2: Mixed valid + invalid user ID — warn log shows structured label

Test 3: All invalid user IDs — bail refuses to start

Uh oh!

masami-agent commented Apr 9, 2026

Uh oh!

masami-agent commented Apr 9, 2026

Uh oh!

masami-agent commented Apr 9, 2026

Uh oh!

This comment was marked as duplicate.

Uh oh!

thepagent left a comment

Choose a reason for hiding this comment

Uh oh!

masami-agent commented Apr 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

masami-agent commented Apr 7, 2026 •

edited

Loading

Testing Results for `parse_id_set()` fix (`9821f3f`)