fix(config): coerce list/dict/model values in ConfigTool instead of storing raw strings#326
Open
sridhar-3009 wants to merge 1 commit into
Open
Conversation
…toring raw strings ConfigTool.execute() only coerced bool/int/float when setting a field. For list, dict, and nested Pydantic model fields the raw input string was stored directly via setattr, causing TypeError or silent misbehavior when downstream code tried to iterate over what it expected to be a list. Add explicit branches for each missing type: - dict: JSON-parse the input and reject non-object payloads with a clear error message. - list: if the input starts with '[' it is parsed as a JSON array; otherwise it is split on commas (e.g. "10.0.0.0/8, 192.168.0.0/16" → ["10.0.0.0/8", "192.168.0.0/16"]) for a convenient shorthand. - BaseModel subfields: JSON-parse and validate via model_validate() so nested settings blocks (e.g. sandbox, web) can be set atomically. Fixes HKUDS#298
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
1 participant
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
Closes #298
ConfigTool.execute()only coercedbool,int, andfloatwhen themodel sent a
setaction. Forlist,dict, and nested Pydantic modelfields the raw input string was stored directly via
setattr, causingTypeErroror silent misbehavior downstream (e.g. code iterating overwhat it expected to be a list, but instead got a string).
New branches added:
dictlist[; otherwise split on commas ("a, b"→["a", "b"])BaseModelsubfieldmodel_validate()Example (before):
Example (after):
Test plan
set web.synthetic_dns_cidrsto"10.0.0.0/8, 192.168.0.0/16"— verify stored value is["10.0.0.0/8", "192.168.0.0/16"]set web.synthetic_dns_cidrsto'["10.0.0.0/8"]'(JSON array form) — verify stored value is["10.0.0.0/8"]set sandbox.docker.extra_envto'{"KEY": "val"}'— verify stored value is{"KEY": "val"}set sandbox.docker.extra_envto an invalid JSON string — verify an error is returned instead of storing raw stringbool/int/floatcoercions continue to work