cleveragents/cleveragents-core

Fork 0

Security issues in CleverAgents... #7

New issue

Open

opened 2025-10-06 01:01:43 +00:00 by brent.edwards · 1 comment

brent.edwards commented

2025-10-06 01:01:43 +00:00

Member

Here are some security problems found within commit 4947b17521, which is the current master branch of cleveragents-core.

These are only POSSIBLE security problems. I have not done extensive analysis. If you say that these security problems can't be exploited, then that should be enough. Just look them over.

(And if you want to complain that I included many security problems that I shouldn't have... ask me for the conversation that I had with Aider to create this list.)

I tried to include all instances of a problem, but I don't promise that I have.

agents/base.py

One basic memory lock is used in AgentWithMemory for _process_wrapper, update_memory, and get_memory.

I don't think Lock is a reentrant lock. If _process_wrapper calls a method that eventually calls get_memory, would there be a deadlock?

agents/composite.py

claude-3.7-sonnet reports the following:

Component Isolation (High Risk)

• The CompositeAgent combines multiple agents, graphs, and streams without clear isolation boundaries.
• If one component is compromised, it could potentially affect others within the same composite.
• Recommendation: Implement stronger isolation between components, possibly using separate execution contexts or sandboxing.

agents/llm.py

If we ever allow outside programs as agents here, we should be much more careful with self.config.get("api_key"). For now, that should be okay.

agents/tool.py

From bandit:

>> Issue: [B404:blacklist] Consider possible security implications associated with the subprocess module.
   Severity: Low   Confidence: High
   CWE: CWE-78 (https://cwe.mitre.org/data/definitions/78.html)
   More Info: https://bandit.readthedocs.io/en/1.8.5/blacklists/blacklist_imports.html#b404-import-subprocess
   Location: agents/tool.py:11:0
10      import logging
11      import subprocess
12      import sys

--------------------------------------------------
>> Issue: [B307:blacklist] Use of possibly insecure function - consider using safer ast.literal_eval.
   Severity: Medium   Confidence: High
   CWE: CWE-78 (https://cwe.mitre.org/data/definitions/78.html)
   More Info: https://bandit.readthedocs.io/en/1.8.5/blacklists/blacklist_calls.html#b307-eval
   Location: agents/tool.py:228:21
227                 }
228                 result = eval(expression, allowed_names)
229                 return str(result)

The eval function is critical to fix.

Lines 153-159 read:

        if self.safe_mode:
            # Basic safety checks
            dangerous_commands = ["rm", "del", "format", "shutdown", "reboot", "kill"]
            if any(cmd in command.lower() for cmd in dangerous_commands):
                raise ExecutionError(
                    f"Dangerous command '{command}' blocked in safe mode"
                )

This is emphatically not enough security. If I recall correctly, you wrote a bunch of examples of bad things that could be done. You should change this code so that bad examples do not get through.

This part of the code is going to be very, very important when the software goes out. It should be easy to change and modify and fix.

The method _http_request_tool makes me very nervous. It can be used to get any http resource that the agent can reach. In particular:

If the agent is run inside a private network but accessible from the Internet, it could return any files from inside the network. (This was a real problem for Amazon.) (Server-side request forgery)
The agent could be used to send malicious requests to other services or used in a denial-of-service attack from our servers.
The agent could request our IAM security credentials.

Lines 277-285 read:

        if self.safe_mode:
            # Always block directory traversal attempts
            if ".." in filepath:
                raise ExecutionError("Unsafe file path blocked in safe mode")
            # Block absolute paths unless in unsafe mode
            if filepath.startswith("/") and not (
                context and context.get("_unsafe_mode", False)
            ):
                raise ExecutionError("Unsafe file path blocked in safe mode")

I don't think that's enough security for safe mode. It doesn't stop safe mode from following soft links. If the user could create a soft link to /, then the whole file system could be read from safe mode.

There's the chance that this code will run on Windows. The filepath should be compared against both / and drive letters in Windows.

langgraph/nodes.py

From bandit:

--------------------------------------------------
>> Issue: [B112:try_except_continue] Try, Except, Continue detected.
   Severity: Low   Confidence: High
   CWE: CWE-703 (https://cwe.mitre.org/data/definitions/703.html)
   More Info: https://bandit.readthedocs.io/en/1.8.5/plugins/b112_try_except_continue.html
   Location: langgraph/nodes.py:128:20
127                             return await self.execute(state)
128                         except Exception:
129                             continue
130

reactive/route_bridge.py

Lines 87-91 are:

        if "custom_predicate" in conditions:
            # Evaluate custom predicate function
            predicate = conditions["custom_predicate"]
            if callable(predicate):
                return predicate(message, route_config)

If route_config.bridge.upgrade_conditions could have come from the user or the LLM, then the code will execute untrusted code.

reactive/stream_router.py

Lines 223-225 are:

            elif "function" in params:
                func_name = params["function"]
                return ops.map(getattr(self, f"_builtin_{func_name}", lambda x: x))

Although this tries to limit code to built-in functions, some [built-in functions](https://docs.python.org/3/library/functions.htmnl should give immediate pause:

breakpoint
compile
eval
exec
open

All of those should be obviously bad to run.

templates/base.py

Lines 234-235 are:

                    template = Template(config)
                    rendered = template.render(**params)

If there is any chance that the parameters came from the user or an LLM, then there are some worries.

See K000152186: Python Jinja2 vulnerability CVE-2025-27516

Also look at Server Side Template Injection with Jinja2 for more security problems that can happen with Jinja and user-controlled settings.

From bandit:

--------------------------------------------------
>> Issue: [B110:try_except_pass] Try, Except, Pass detected.
   Severity: Low   Confidence: High
   CWE: CWE-703 (https://cwe.mitre.org/data/definitions/703.html)
   More Info: https://bandit.readthedocs.io/en/1.8.5/plugins/b110_try_except_pass.html
   Location: templates/base.py:250:24
249                                 return json.loads(json_str)
250                             except:
251                                 pass
252

templates/deferred_template.py

Line 66 reads:

        section_yaml = yaml.dump({key: yaml_dict[key]}, default_flow_style=False)

Line 96 reads:

            section_yaml = yaml.dump(processed[section], default_flow_style=False)`

Although it is not as important as yaml.safe_load, code should use yaml.safe_dump instead of yaml.dump. According to PyYAML Documentation.

safe_dump(data, stream=None) serializes the given Python object into the stream. If stream is None, it returns the produced stream. safe_dump produces only standard YAML tags and cannot represent an arbitrary Python object.

templates/enhanced_registry.py

Line 97 is:

        yaml_str = yaml.dump(definition, default_flow_style=False)

Same comment about using yaml.safe_dump instead of yaml.dump

templates/graph_templates.py

From bandit:

--------------------------------------------------
>> Issue: [B110:try_except_pass] Try, Except, Pass detected.
   Severity: Low   Confidence: High
   CWE: CWE-703 (https://cwe.mitre.org/data/definitions/703.html)
   More Info: https://bandit.readthedocs.io/en/1.8.5/plugins/b110_try_except_pass.html
   Location: templates/graph_templates.py:93:20
92                                  )
93                          except:
94                              # Not a local reference, probably a global agent name
95                              pass
96

templates/inline_jinja_handler.py

From bandit:

--------------------------------------------------
>> Issue: [B701:jinja2_autoescape_false] By default, jinja2 sets autoescape to False. Consider using autoescape=True or use the select_autoescape function to mitigate XSS vulnerabilities.
   Severity: High   Confidence: High
   CWE: CWE-94 (https://cwe.mitre.org/data/definitions/94.html)
   More Info: https://bandit.readthedocs.io/en/1.8.5/plugins/b701_jinja2_autoescape_false.html
   Location: templates/inline_jinja_handler.py:38:19
37          def __init__(self):
38              self.env = Environment(
39                  block_start_string="{%",
40                  block_end_string="%}",
41                  variable_start_string="{{",
42                  variable_end_string="}}",
43                  comment_start_string="{#",
44                  comment_end_string="#}",
45                  trim_blocks=True,
46                  lstrip_blocks=True,
47              )
48

--------------------------------------------------
>> Issue: [B701:jinja2_autoescape_false] By default, jinja2 sets autoescape to False. Consider using autoescape=True or use the select_autoescape function to mitigate XSS vulnerabilities.
   Severity: High   Confidence: High
   CWE: CWE-94 (https://cwe.mitre.org/data/definitions/94.html)
   More Info: https://bandit.readthedocs.io/en/1.8.5/plugins/b701_jinja2_autoescape_false.html
   Location: templates/inline_yaml_jinja.py:38:19
37              # Configure Jinja2 to work well with YAML
38              self.env = Environment(
39                  block_start_string="{%",
40                  block_end_string="%}",
41                  variable_start_string="{{",
42                  variable_end_string="}}",
43                  comment_start_string="{#",
44                  comment_end_string="#}",
45              )
46

templates/inline_yaml_jinja.py

Lines 119-120 are:

            template = self.env.from_string(content)
            rendered = template.render(**full_context)

If the content or context came from the user, then Jinja2 might be able to execute Python expressions.

templates/jinja_yaml_preprocessor.py:

From bandit:

--------------------------------------------------
>> Issue: [B701:jinja2_autoescape_false] By default, jinja2 sets autoescape to False. Consider using autoescape=True or use the select_autoescape function to mitigate XSS vulnerabilities.
   Severity: High   Confidence: High
   CWE: CWE-94 (https://cwe.mitre.org/data/definitions/94.html)
   More Info: https://bandit.readthedocs.io/en/1.8.5/plugins/b701_jinja2_autoescape_false.html
   Location: templates/jinja_yaml_preprocessor.py:35:19
34          def __init__(self):
35              self.env = Environment(
36                  block_start_string="{%",
37                  block_end_string="%}",
38                  variable_start_string="{{",
39                  variable_end_string="}}",
40                  comment_start_string="{#",
41                  comment_end_string="#}",
42                  trim_blocks=True,
43                  lstrip_blocks=True,
44              )
45

Lines 94-95 are:

        template = self.env.from_string(yaml_content)
        rendered = template.render(**full_context)

Same problem with Jinja templates.

templates/registry.py

Lines 107-108 are:

                    template = self.get_template(template_type, template_name)
                    return template.instantiate(params, self, context)

Same problem with Jinja templates.

templates/renderer.py:

Lines 98-100 are:

                self.engine = jinja2.Environment(
                    autoescape=False, trim_blocks=True, lstrip_blocks=True
                )

According to Python Jinja2: always autoescape to avoid XSS attacks, if autoescape is false, then HTML won't be escaped, so there may be XSS vulnerabilities.

From bandit:

--------------------------------------------------
>> Issue: [B701:jinja2_autoescape_false] Using jinja2 templates with autoescape=False is dangerous and can lead to XSS. Use autoescape=True or use the select_autoescape function to mitigate XSS vulnerabilities.
   Severity: High   Confidence: High
   CWE: CWE-94 (https://cwe.mitre.org/data/definitions/94.html)
   More Info: https://bandit.readthedocs.io/en/1.8.5/plugins/b701_jinja2_autoescape_false.html
   Location: templates/renderer.py:98:30
97
98                      self.engine = jinja2.Environment(
99                          autoescape=False, trim_blocks=True, lstrip_blocks=True
100                     )
101                 except ImportError:

--------------------------------------------------
>> Issue: [B307:blacklist] Use of possibly insecure function - consider using safer ast.literal_eval.
   Severity: Medium   Confidence: High
   CWE: CWE-78 (https://cwe.mitre.org/data/definitions/78.html)
   More Info: https://bandit.readthedocs.io/en/1.8.5/blacklists/blacklist_calls.html#b307-eval
   Location: templates/renderer.py:129:20
128                 # This enables support for constructs like `context.get('key', default)`
129                 value = eval(expr, {"__builtins__": {}}, context)  # noqa: S307
130                 return "" if value is None else str(value)

There are multiple places where the code processes templates with (possibly) user-supplied values. If that's the case, then there is a security vulnerabitily.

templates/smart_yaml_loader.py

Not a security problem, but the parsing code in lines 73-137 cannot handle nested for instructions.

Lines 274-275 read:

        processor = YAMLTemplateProcessor()
        return processor.process_string(yaml_str, context)

If user-controlled input is included, this might cause template injection vulnerabilities.

templates/stream_templates.py:

From bandit:

--------------------------------------------------
>> Issue: [B110:try_except_pass] Try, Except, Pass detected.
   Severity: Low   Confidence: High
   CWE: CWE-703 (https://cwe.mitre.org/data/definitions/703.html)
   More Info: https://bandit.readthedocs.io/en/1.8.5/plugins/b110_try_except_pass.html
   Location: templates/stream_templates.py:109:20
108                                 )
109                         except:
110                             # Not a local reference
111                             pass
112

--------------------------------------------------
>> Issue: [B110:try_except_pass] Try, Except, Pass detected.
   Severity: Low   Confidence: High
   CWE: CWE-703 (https://cwe.mitre.org/data/definitions/703.html)
   More Info: https://bandit.readthedocs.io/en/1.8.5/plugins/b110_try_except_pass.html
   Location: templates/stream_templates.py:131:20
130                                 )
131                         except:
132                             # Not a local reference
133                             pass
134

templates/template_store.py

Line 65 is:

            yaml_str = yaml.dump(definition, default_flow_style=False)

and line 154 is:

            self.raw_yaml = yaml.dump(definition, default_flow_style=False)

They should be yaml.safe_dump.

Lines 114-119 have a set of functions -- but they're different than other sets of functions. The functions should be unified.

Line 124 reads:

        result = processor.process_string(raw_template, context)

This might be a security problem.

templates/yaml_jinja_loader.py

From bandit:

>> Issue: [B701:jinja2_autoescape_false] By default, jinja2 sets autoescape to False. Consider using autoescape=True or use the select_autoescape function to mitigate XSS vulnerabilities.
   Severity: High   Confidence: High
   CWE: CWE-94 (https://cwe.mitre.org/data/definitions/94.html)
   More Info: https://bandit.readthedocs.io/en/1.8.5/plugins/b701_jinja2_autoescape_false.html
   Location: templates/yaml_jinja_loader.py:33:19
32          def __init__(self):
33              self.env = Environment(
34                  block_start_string="{%",
35                  block_end_string="%}",
36                  variable_start_string="{{",
37                  variable_end_string="}}",
38                  comment_start_string="{#",
39                  comment_end_string="#}",
40                  trim_blocks=True,
41                  lstrip_blocks=True,
42              )
43

If yaml_content or context could have come from users, then there might be security issues with lines 126-127.

            template = self.env.from_string(yaml_content)
            rendered = template.render(**context)

The utility functions in lines 80-91 are different than utility functions in other parts of the code. There should be just one master list of utility functions.

templates/yaml_preprocessor.py

From bandit:

--------------------------------------------------
>> Issue: [B701:jinja2_autoescape_false] By default, jinja2 sets autoescape to False. Consider using autoescape=True or use the select_autoescape function to mitigate XSS vulnerabilities.
   Severity: High   Confidence: High
   CWE: CWE-94 (https://cwe.mitre.org/data/definitions/94.html)
   More Info: https://bandit.readthedocs.io/en/1.8.5/plugins/b701_jinja2_autoescape_false.html
   Location: templates/yaml_preprocessor.py:34:19
33          def __init__(self):
34              self.env = Environment(
35                  block_start_string="{%",
36                  block_end_string="%}",
37                  variable_start_string="{{",
38                  variable_end_string="}}",
39                  comment_start_string="{#",
40                  comment_end_string="#}",
41                  trim_blocks=True,
42                  lstrip_blocks=True,
43              )
44

templates/yaml_template_engine.py

From bandit:

--------------------------------------------------
>> Issue: [B701:jinja2_autoescape_false] By default, jinja2 sets autoescape to False. Consider using autoescape=True or use the select_autoescape function to mitigate XSS vulnerabilities.
   Severity: High   Confidence: High
   CWE: CWE-94 (https://cwe.mitre.org/data/definitions/94.html)
   More Info: https://bandit.readthedocs.io/en/1.8.5/plugins/b701_jinja2_autoescape_false.html
   Location: templates/yaml_template_engine.py:37:19
36              # Configure Jinja2 for YAML-friendly output
37              self.env = Environment(
38                  block_start_string="{%",
39                  block_end_string="%}",
40                  variable_start_string="{{",
41                  variable_end_string="}}",
42                  comment_start_string="{#",
43                  comment_end_string="#}",
44                  trim_blocks=False,  # Don't trim to preserve structure
45                  lstrip_blocks=False,  # Don't strip to preserve indentation
46                  keep_trailing_newline=True,
47              )
48

Lines 76-77 are:

            template = self.env.from_string(yaml_content)
            processed_content = template.render(**context)

This has the same problems with YAML as listed above.

templates/yaml_template_engine.py

Lines 97-99 are:

        # Render as Jinja2 template
        template = self.env.from_string(processed_content)
        rendered = template.render(**full_context)

This has the same problems with YAML as listed above.

Lines 388-407 contain a different list of context functions than elsewhere. There should be just one list of functions, used everywhere.

cli.py

Lines 87-89 are:

    if output:
        with output.open("w") as f:
            f.write(result)

They will overwrite any file that the user gives. If the program has access to files that the user does not, this might be used to destroy files that the user otherwise doesn't have access to.

Here are some security problems found within commit 4947b17521482615d87, which is the current `master` branch of cleveragents-core. These are only POSSIBLE security problems. I have not done extensive analysis. If you say that these security problems can't be exploited, then that should be enough. Just look them over. (And if you want to complain that I included many security problems that I shouldn't have... ask me for the conversation that I had with Aider to create this list.) I tried to include all instances of a problem, but I don't promise that I have. # agents/base.py One basic memory lock is used in `AgentWithMemory` for `_process_wrapper`, `update_memory`, and `get_memory`. I don't think `Lock` is a reentrant lock. If `_process_wrapper` calls a method that eventually calls `get_memory`, would there be a deadlock? # agents/composite.py `claude-3.7-sonnet` reports the following: Component Isolation (High Risk) • The CompositeAgent combines multiple agents, graphs, and streams without clear isolation boundaries. • If one component is compromised, it could potentially affect others within the same composite. • Recommendation: Implement stronger isolation between components, possibly using separate execution contexts or sandboxing. # agents/llm.py If we ever allow outside programs as agents here, we should be much more careful with `self.config.get("api_key")`. For now, that should be okay. # agents/tool.py From bandit: ``` >> Issue: [B404:blacklist] Consider possible security implications associated with the subprocess module. Severity: Low Confidence: High CWE: CWE-78 (https://cwe.mitre.org/data/definitions/78.html) More Info: https://bandit.readthedocs.io/en/1.8.5/blacklists/blacklist_imports.html#b404-import-subprocess Location: agents/tool.py:11:0 10 import logging 11 import subprocess 12 import sys -------------------------------------------------- >> Issue: [B307:blacklist] Use of possibly insecure function - consider using safer ast.literal_eval. Severity: Medium Confidence: High CWE: CWE-78 (https://cwe.mitre.org/data/definitions/78.html) More Info: https://bandit.readthedocs.io/en/1.8.5/blacklists/blacklist_calls.html#b307-eval Location: agents/tool.py:228:21 227 } 228 result = eval(expression, allowed_names) 229 return str(result) ``` The `eval` function is critical to fix. Lines 153-159 read: ``` if self.safe_mode: # Basic safety checks dangerous_commands = ["rm", "del", "format", "shutdown", "reboot", "kill"] if any(cmd in command.lower() for cmd in dangerous_commands): raise ExecutionError( f"Dangerous command '{command}' blocked in safe mode" ) ``` This is emphatically not enough security. If I recall correctly, you wrote a bunch of examples of bad things that could be done. You should change this code so that bad examples do not get through. This part of the code is going to be very, very important when the software goes out. It should be easy to change and modify and fix. --- The method `_http_request_tool` makes me very nervous. It can be used to get any http resource that the agent can reach. In particular: 1. If the agent is run inside a private network but accessible from the Internet, it could return any files from inside the network. (This was a real problem for Amazon.) (Server-side request forgery) 2. The agent could be used to send malicious requests to other services or used in a denial-of-service attack from our servers. 3. The agent could request our IAM security credentials. --- Lines 277-285 read: ``` if self.safe_mode: # Always block directory traversal attempts if ".." in filepath: raise ExecutionError("Unsafe file path blocked in safe mode") # Block absolute paths unless in unsafe mode if filepath.startswith("/") and not ( context and context.get("_unsafe_mode", False) ): raise ExecutionError("Unsafe file path blocked in safe mode") ``` I don't think that's enough security for safe mode. It doesn't stop safe mode from following soft links. If the user could create a soft link to `/`, then the whole file system could be read from safe mode. There's the chance that this code will run on Windows. The filepath should be compared against both `/` and drive letters in Windows. # langgraph/nodes.py From bandit: ``` -------------------------------------------------- >> Issue: [B112:try_except_continue] Try, Except, Continue detected. Severity: Low Confidence: High CWE: CWE-703 (https://cwe.mitre.org/data/definitions/703.html) More Info: https://bandit.readthedocs.io/en/1.8.5/plugins/b112_try_except_continue.html Location: langgraph/nodes.py:128:20 127 return await self.execute(state) 128 except Exception: 129 continue 130 ``` # reactive/route_bridge.py Lines 87-91 are: ``` if "custom_predicate" in conditions: # Evaluate custom predicate function predicate = conditions["custom_predicate"] if callable(predicate): return predicate(message, route_config) ``` If `route_config.bridge.upgrade_conditions` could have come from the user or the LLM, then the code will execute untrusted code. # reactive/stream_router.py Lines 223-225 are: ``` elif "function" in params: func_name = params["function"] return ops.map(getattr(self, f"_builtin_{func_name}", lambda x: x)) ``` Although this tries to limit code to built-in functions, some [built-in functions](https://docs.python.org/3/library/functions.htmnl should give immediate pause: - `breakpoint` - `compile` - `eval` - `exec` - `open` All of those should be obviously bad to run. # templates/base.py Lines 234-235 are: ``` template = Template(config) rendered = template.render(**params) ``` If there is any chance that the parameters came from the user or an LLM, then there are some worries. See [K000152186: Python Jinja2 vulnerability CVE-2025-27516](https://my.f5.com/manage/s/article/K000152186) Also look at [Server Side Template Injection with Jinja2](https://onsecurity.io/article/server-side-template-injection-with-jinja2/) for more security problems that can happen with Jinja and user-controlled settings. --- From bandit: ``` -------------------------------------------------- >> Issue: [B110:try_except_pass] Try, Except, Pass detected. Severity: Low Confidence: High CWE: CWE-703 (https://cwe.mitre.org/data/definitions/703.html) More Info: https://bandit.readthedocs.io/en/1.8.5/plugins/b110_try_except_pass.html Location: templates/base.py:250:24 249 return json.loads(json_str) 250 except: 251 pass 252 ``` # templates/deferred_template.py Line 66 reads: ``` section_yaml = yaml.dump({key: yaml_dict[key]}, default_flow_style=False) ``` Line 96 reads: ``` section_yaml = yaml.dump(processed[section], default_flow_style=False)` ``` Although it is not as important as `yaml.safe_load`, code should use `yaml.safe_dump` instead of `yaml.dump`. According to [PyYAML Documentation](https://pyyaml.org/wiki/PyYAMLDocumentation). > safe_dump(data, stream=None) serializes the given Python object into the stream. If stream is None, it returns the produced stream. safe_dump produces only standard YAML tags and cannot represent an arbitrary Python object. # templates/enhanced_registry.py Line 97 is: ``` yaml_str = yaml.dump(definition, default_flow_style=False) ``` Same comment about using `yaml.safe_dump` instead of `yaml.dump` # templates/graph_templates.py From bandit: ``` -------------------------------------------------- >> Issue: [B110:try_except_pass] Try, Except, Pass detected. Severity: Low Confidence: High CWE: CWE-703 (https://cwe.mitre.org/data/definitions/703.html) More Info: https://bandit.readthedocs.io/en/1.8.5/plugins/b110_try_except_pass.html Location: templates/graph_templates.py:93:20 92 ) 93 except: 94 # Not a local reference, probably a global agent name 95 pass 96 ``` # templates/inline_jinja_handler.py From bandit: ``` -------------------------------------------------- >> Issue: [B701:jinja2_autoescape_false] By default, jinja2 sets autoescape to False. Consider using autoescape=True or use the select_autoescape function to mitigate XSS vulnerabilities. Severity: High Confidence: High CWE: CWE-94 (https://cwe.mitre.org/data/definitions/94.html) More Info: https://bandit.readthedocs.io/en/1.8.5/plugins/b701_jinja2_autoescape_false.html Location: templates/inline_jinja_handler.py:38:19 37 def __init__(self): 38 self.env = Environment( 39 block_start_string="{%", 40 block_end_string="%}", 41 variable_start_string="{{", 42 variable_end_string="}}", 43 comment_start_string="{#", 44 comment_end_string="#}", 45 trim_blocks=True, 46 lstrip_blocks=True, 47 ) 48 -------------------------------------------------- >> Issue: [B701:jinja2_autoescape_false] By default, jinja2 sets autoescape to False. Consider using autoescape=True or use the select_autoescape function to mitigate XSS vulnerabilities. Severity: High Confidence: High CWE: CWE-94 (https://cwe.mitre.org/data/definitions/94.html) More Info: https://bandit.readthedocs.io/en/1.8.5/plugins/b701_jinja2_autoescape_false.html Location: templates/inline_yaml_jinja.py:38:19 37 # Configure Jinja2 to work well with YAML 38 self.env = Environment( 39 block_start_string="{%", 40 block_end_string="%}", 41 variable_start_string="{{", 42 variable_end_string="}}", 43 comment_start_string="{#", 44 comment_end_string="#}", 45 ) 46 ``` # templates/inline_yaml_jinja.py Lines 119-120 are: ``` template = self.env.from_string(content) rendered = template.render(**full_context) ``` If the `content` or `context` came from the user, then Jinja2 might be able to execute Python expressions. # templates/jinja_yaml_preprocessor.py: From bandit: ``` -------------------------------------------------- >> Issue: [B701:jinja2_autoescape_false] By default, jinja2 sets autoescape to False. Consider using autoescape=True or use the select_autoescape function to mitigate XSS vulnerabilities. Severity: High Confidence: High CWE: CWE-94 (https://cwe.mitre.org/data/definitions/94.html) More Info: https://bandit.readthedocs.io/en/1.8.5/plugins/b701_jinja2_autoescape_false.html Location: templates/jinja_yaml_preprocessor.py:35:19 34 def __init__(self): 35 self.env = Environment( 36 block_start_string="{%", 37 block_end_string="%}", 38 variable_start_string="{{", 39 variable_end_string="}}", 40 comment_start_string="{#", 41 comment_end_string="#}", 42 trim_blocks=True, 43 lstrip_blocks=True, 44 ) 45 ``` Lines 94-95 are: ``` template = self.env.from_string(yaml_content) rendered = template.render(**full_context) ``` Same problem with Jinja templates. # templates/registry.py Lines 107-108 are: ``` template = self.get_template(template_type, template_name) return template.instantiate(params, self, context) ``` Same problem with Jinja templates. # templates/renderer.py: Lines 98-100 are: ``` self.engine = jinja2.Environment( autoescape=False, trim_blocks=True, lstrip_blocks=True ) ``` According to [Python Jinja2: always autoescape to avoid XSS attacks](https://www.codiga.io/blog/python-jinja2-autoescape/), if `autoescape` is false, then HTML won't be escaped, so there may be XSS vulnerabilities. --- From bandit: ``` -------------------------------------------------- >> Issue: [B701:jinja2_autoescape_false] Using jinja2 templates with autoescape=False is dangerous and can lead to XSS. Use autoescape=True or use the select_autoescape function to mitigate XSS vulnerabilities. Severity: High Confidence: High CWE: CWE-94 (https://cwe.mitre.org/data/definitions/94.html) More Info: https://bandit.readthedocs.io/en/1.8.5/plugins/b701_jinja2_autoescape_false.html Location: templates/renderer.py:98:30 97 98 self.engine = jinja2.Environment( 99 autoescape=False, trim_blocks=True, lstrip_blocks=True 100 ) 101 except ImportError: -------------------------------------------------- >> Issue: [B307:blacklist] Use of possibly insecure function - consider using safer ast.literal_eval. Severity: Medium Confidence: High CWE: CWE-78 (https://cwe.mitre.org/data/definitions/78.html) More Info: https://bandit.readthedocs.io/en/1.8.5/blacklists/blacklist_calls.html#b307-eval Location: templates/renderer.py:129:20 128 # This enables support for constructs like `context.get('key', default)` 129 value = eval(expr, {"__builtins__": {}}, context) # noqa: S307 130 return "" if value is None else str(value) ``` --- There are multiple places where the code processes templates with (possibly) user-supplied values. If that's the case, then there is a security vulnerabitily. # templates/smart_yaml_loader.py Not a security problem, but the parsing code in lines 73-137 cannot handle nested `for` instructions. Lines 274-275 read: ``` processor = YAMLTemplateProcessor() return processor.process_string(yaml_str, context) ``` If user-controlled input is included, this might cause template injection vulnerabilities. # templates/stream_templates.py: From bandit: ``` -------------------------------------------------- >> Issue: [B110:try_except_pass] Try, Except, Pass detected. Severity: Low Confidence: High CWE: CWE-703 (https://cwe.mitre.org/data/definitions/703.html) More Info: https://bandit.readthedocs.io/en/1.8.5/plugins/b110_try_except_pass.html Location: templates/stream_templates.py:109:20 108 ) 109 except: 110 # Not a local reference 111 pass 112 -------------------------------------------------- >> Issue: [B110:try_except_pass] Try, Except, Pass detected. Severity: Low Confidence: High CWE: CWE-703 (https://cwe.mitre.org/data/definitions/703.html) More Info: https://bandit.readthedocs.io/en/1.8.5/plugins/b110_try_except_pass.html Location: templates/stream_templates.py:131:20 130 ) 131 except: 132 # Not a local reference 133 pass 134 ``` # templates/template_store.py Line 65 is: ``` yaml_str = yaml.dump(definition, default_flow_style=False) ``` and line 154 is: ``` self.raw_yaml = yaml.dump(definition, default_flow_style=False) ``` They should be `yaml.safe_dump`. --- Lines 114-119 have a set of functions -- but they're different than other sets of functions. The functions should be unified. --- Line 124 reads: ``` result = processor.process_string(raw_template, context) ``` This might be a security problem. # templates/yaml_jinja_loader.py From bandit: ``` >> Issue: [B701:jinja2_autoescape_false] By default, jinja2 sets autoescape to False. Consider using autoescape=True or use the select_autoescape function to mitigate XSS vulnerabilities. Severity: High Confidence: High CWE: CWE-94 (https://cwe.mitre.org/data/definitions/94.html) More Info: https://bandit.readthedocs.io/en/1.8.5/plugins/b701_jinja2_autoescape_false.html Location: templates/yaml_jinja_loader.py:33:19 32 def __init__(self): 33 self.env = Environment( 34 block_start_string="{%", 35 block_end_string="%}", 36 variable_start_string="{{", 37 variable_end_string="}}", 38 comment_start_string="{#", 39 comment_end_string="#}", 40 trim_blocks=True, 41 lstrip_blocks=True, 42 ) 43 ``` --- If `yaml_content` or `context` could have come from users, then there might be security issues with lines 126-127. ``` template = self.env.from_string(yaml_content) rendered = template.render(**context) ```` --- The utility functions in lines 80-91 are different than utility functions in other parts of the code. There should be just one master list of utility functions. # templates/yaml_preprocessor.py From bandit: ``` -------------------------------------------------- >> Issue: [B701:jinja2_autoescape_false] By default, jinja2 sets autoescape to False. Consider using autoescape=True or use the select_autoescape function to mitigate XSS vulnerabilities. Severity: High Confidence: High CWE: CWE-94 (https://cwe.mitre.org/data/definitions/94.html) More Info: https://bandit.readthedocs.io/en/1.8.5/plugins/b701_jinja2_autoescape_false.html Location: templates/yaml_preprocessor.py:34:19 33 def __init__(self): 34 self.env = Environment( 35 block_start_string="{%", 36 block_end_string="%}", 37 variable_start_string="{{", 38 variable_end_string="}}", 39 comment_start_string="{#", 40 comment_end_string="#}", 41 trim_blocks=True, 42 lstrip_blocks=True, 43 ) 44 ``` # templates/yaml_template_engine.py From bandit: ``` -------------------------------------------------- >> Issue: [B701:jinja2_autoescape_false] By default, jinja2 sets autoescape to False. Consider using autoescape=True or use the select_autoescape function to mitigate XSS vulnerabilities. Severity: High Confidence: High CWE: CWE-94 (https://cwe.mitre.org/data/definitions/94.html) More Info: https://bandit.readthedocs.io/en/1.8.5/plugins/b701_jinja2_autoescape_false.html Location: templates/yaml_template_engine.py:37:19 36 # Configure Jinja2 for YAML-friendly output 37 self.env = Environment( 38 block_start_string="{%", 39 block_end_string="%}", 40 variable_start_string="{{", 41 variable_end_string="}}", 42 comment_start_string="{#", 43 comment_end_string="#}", 44 trim_blocks=False, # Don't trim to preserve structure 45 lstrip_blocks=False, # Don't strip to preserve indentation 46 keep_trailing_newline=True, 47 ) 48 ``` Lines 76-77 are: ``` template = self.env.from_string(yaml_content) processed_content = template.render(**context) ``` This has the same problems with YAML as listed above. # templates/yaml_template_engine.py Lines 97-99 are: ``` # Render as Jinja2 template template = self.env.from_string(processed_content) rendered = template.render(**full_context) ``` This has the same problems with YAML as listed above. ---- Lines 388-407 contain a different list of context functions than elsewhere. There should be just one list of functions, used everywhere. # cli.py Lines 87-89 are: ``` if output: with output.open("w") as f: f.write(result) ``` They will overwrite any file that the user gives. If the program has access to files that the user does not, this might be used to destroy files that the user otherwise doesn't have access to.

brent.edwards commented

2025-10-29 02:43:31 +00:00

Author

Member

Here is the complete conversation between Aider and I for the security code review.

aider.txt

348 KiB