fix: strip whitespace and handle null values in instance configuration#9239
fix: strip whitespace and handle null values in instance configuration#9239okxint wants to merge 1 commit into
Conversation
When patching instance configuration values, the raw values from request.data were used directly without sanitization. This adds: - Whitespace stripping via str().strip() to prevent leading/trailing spaces from being stored - Explicit None handling so that null values become empty strings instead of the literal string "None"
📝 WalkthroughWalkthroughIn ChangesConfiguration Value Normalization
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/plane/license/api/views/configuration.py`:
- Around line 48-49: In the configuration sanitization logic where raw_value is
converted to a string using str(), add explicit handling for boolean values.
After the existing line that strips the string value, check if raw_value is a
boolean type using isinstance() and convert it to "1" for True or "0" for False
to match the expected string format throughout the codebase. This ensures that
boolean flag configurations like EMAIL_USE_TLS and EMAIL_USE_SSL are properly
converted to the string representations they are compared against in equality
checks, preventing failed validations when the frontend sends actual boolean
types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0a0ab350-19e8-4ada-8b2a-e46fdffada78
📒 Files selected for processing (1)
apps/api/plane/license/api/views/configuration.py
| raw_value = request.data.get(configuration.key, configuration.value) | ||
| value = "" if raw_value is None else str(raw_value).strip() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for configuration keys that are compared to "0" or "1" strings
rg -n -C2 'EMAIL_USE_TLS|EMAIL_USE_SSL|ENABLE_SMTP' --type=py
# Search for any validation or serializer logic for InstanceConfiguration
ast-grep --pattern 'class InstanceConfigurationSerializer($$$) {
$$$
}'
# Check if request.data typing or validation constrains boolean flags
rg -n 'InstanceConfiguration.*patch|def patch.*InstanceConfiguration' --type=py -A 10Repository: makeplane/plane
Length of output: 17116
🏁 Script executed:
# Read the configuration.py file to see the full patch method context
cat -n apps/api/plane/license/api/views/configuration.py | head -60Repository: makeplane/plane
Length of output: 2836
🏁 Script executed:
# Search for InstanceConfiguration model definition
rg -n 'class InstanceConfiguration' --type=py -A 10Repository: makeplane/plane
Length of output: 3171
🏁 Script executed:
# Search for any serializer related to InstanceConfiguration
rg -n 'InstanceConfiguration.*Serializer|class.*Serializer.*InstanceConfiguration' --type=py -A 15Repository: makeplane/plane
Length of output: 5740
🏁 Script executed:
# Check if there's any type conversion or validation for boolean fields
rg -n 'BooleanField|to_internal_value|validate' apps/api/plane/license/api/ --type=py | head -20Repository: makeplane/plane
Length of output: 1715
🏁 Script executed:
# Check if there are any tests or examples showing what format is sent to the patch endpoint
rg -n 'EMAIL_USE_TLS|EMAIL_USE_SSL|ENABLE_SMTP' apps/api/plane/license/api/tests/ --type=py -A 2 -B 2 2>/dev/null || echo "No tests found"Repository: makeplane/plane
Length of output: 74
🏁 Script executed:
# Check frontend/migrations to see if booleans might be sent
fd -e ts -e tsx -e js apps/web 2>/dev/null | head -5Repository: makeplane/plane
Length of output: 41
Handle boolean-to-string conversion for flag-type configurations.
The str() coercion on line 49 converts boolean True/False to strings "True"/"False", but boolean flag configurations like EMAIL_USE_TLS and EMAIL_USE_SSL are compared as "1" or "0" strings throughout the codebase (e.g., use_tls=EMAIL_USE_TLS == "1"). If the frontend sends actual boolean types for these flags, the sanitization produces incorrect string values that fail equality checks.
The InstanceConfigurationSerializer has no custom field definitions or type validation (fields = "__all__"), so booleans pass through unchecked. Add explicit handling to convert boolean values to the expected string format:
value = "" if raw_value is None else str(raw_value).strip()
# For boolean flags, convert to "1" or "0"
if isinstance(raw_value, bool):
value = "1" if raw_value else "0"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/plane/license/api/views/configuration.py` around lines 48 - 49, In
the configuration sanitization logic where raw_value is converted to a string
using str(), add explicit handling for boolean values. After the existing line
that strips the string value, check if raw_value is a boolean type using
isinstance() and convert it to "1" for True or "0" for False to match the
expected string format throughout the codebase. This ensures that boolean flag
configurations like EMAIL_USE_TLS and EMAIL_USE_SSL are properly converted to
the string representations they are compared against in equality checks,
preventing failed validations when the frontend sends actual boolean types.
1 participant
What
The instance configuration PATCH endpoint stored raw values from
request.datawithout sanitization, causing two issues:str(None)produces the literal string"None"instead of an empty string, so clearing a field would store"None"in the DB.Fix
Both issues are addressed in a single sanitization step before the value is encrypted or stored.
Files changed
apps/api/plane/license/api/views/configuration.pySummary by CodeRabbit