fix(helm): escape all user-supplied TOML string values with toJson#426
Merged
thepagent merged 1 commit intoopenabdev:mainfrom Apr 17, 2026
Merged
fix(helm): escape all user-supplied TOML string values with toJson#426thepagent merged 1 commit intoopenabdev:mainfrom
thepagent merged 1 commit intoopenabdev:mainfrom
Conversation
Replace raw string interpolation "{{ $v }}" with {{ $v | toJson }}
across all user-supplied string fields in configmap.yaml:
- env values (injection risk with quotes/backslashes)
- command, working_dir
- discord/slack allow_bot_messages, slack allow_user_messages
- stt model, base_url
toJson produces a valid JSON string (with quotes and escaping), which
is compatible with TOML basic string syntax. Existing values without
special characters render identically.
Fields NOT changed (no user input):
- bot_token, app_token, api_key: use env var placeholders (${...})
- allowed_channels, allowed_users, trusted_bot_ids: already use toJson
- args: already uses toJson
thepagent
approved these changes
Apr 17, 2026
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.
What problem does this solve?
The configmap template renders user-supplied string values with raw interpolation:
If a value contains double quotes or backslashes, the rendered TOML is malformed. For example,
MY_VAR: 'say "hello"'produces:This breaks TOML parsing at runtime. A crafted value could also inject arbitrary TOML keys.
The same unsafe pattern exists in 8 fields across the template:
envvalues,command,working_dir,allow_bot_messages(Discord/Slack),allow_user_messages(Slack), and STTmodel/base_url.Related: #425 (Helm chart analysis report)
Credit
This issue was originally identified and fixed by @thekkagent in #380 as part of a larger Helm chart extensibility PR. This PR extracts the TOML escaping fix as a standalone security patch, and extends it to cover all remaining string fields in the template.
Proposed Solution
Replace all
"{{ $v }}"patterns with{{ $v | toJson }}.toJsonoutputs a valid JSON string (with surrounding quotes and proper escaping). TOML basic string syntax is compatible with JSON strings, so the output is valid TOML.hello"hello""hello"say "hi""say "hi""❌"say \"hi\""✅path\to\file"path\to\file"❌"path\\to\\file"✅Fields changed
envvaluescommandworking_dirallow_bot_messages(Discord)allow_bot_messages(Slack)allow_user_messages(Slack)model(STT)base_url(STT)Fields NOT changed (no user input)
bot_token,app_token,api_key${...}), not user valuesallowed_channels,allowed_users,trusted_bot_idstoJsonargstoJsonWhy this approach?
toJsonis already used elsewhere in this template (forargs,allowed_channels, etc.), so this is consistent with the existing escaping strategy. It produces output identical to the original for values without special characters, making it fully backward compatible.Alternatives Considered
1. Fix only
envvalues — rejectedThe
envfield has the highest risk since it accepts arbitrary user input. However, applying the same fix to all string fields is trivial (same one-line pattern) and eliminates the entire vulnerability class from the template. Leaving some fields unfixed would be inconsistent.2. Use Helm
quotefunction — rejectedquotewraps a value in double quotes but does not escape internal quotes or backslashes. It would not fix the injection.3. Use TOML literal strings (
'...') — rejectedTOML literal strings don't process escape sequences, but they also can't contain single quotes. This would trade one injection vector for another.
Validation
helm lint charts/openab— passed (0 failures)main(no behavioral change)Output:
All fields render correctly with proper quoting.
Scope
One file changed, 8 lines modified. No new features, no new values fields, no template logic changes. Pure security fix.
DC: https://discord.com/channels/1491295327620169908/1493841502529523732