Skip to content
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions mlx/robot2rst/robot2rst.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
''' Script to convert a robot test file to a reStructuredText file with traceable items '''
import argparse
import logging
import os
import sys
from textwrap import indent
from pathlib import Path
Expand Down Expand Up @@ -54,13 +55,15 @@ def generate_robot_2_rst(parser, rst_file, prefix, relationship_config, gen_matr
relationship_config (list): List of tuples that contain a relationship, tag_regex and coverage percentage
gen_matrix (bool): True if traceability matrices are to be generated, False if not.
"""
env_vars = dict(os.environ)
return render_template(
rst_file,
parser=parser,
suite=rst_file.stem,
prefix=prefix,
relationship_config=relationship_config,
gen_matrix=gen_matrix,
**env_vars,
**kwargs,
)
Copy link

@coderabbitai coderabbitai bot Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Namespace collision risk and unnecessary exposure of all environment variables.

Two concerns with unpacking **env_vars directly into the template keyword arguments:

  1. TypeError at runtime if any env-var name collides with an existing keyword argument. If an environment variable happens to be named parser, suite, prefix, gen_matrix, test_type, only, coverages, etc., Python will raise TypeError: got multiple values for keyword argument '…'. While most standard env vars are uppercase, this is not guaranteed—CI systems and users can define arbitrary names.

  2. All environment variables (including secrets) become available to the Mako template. Env vars frequently carry credentials (AWS_SECRET_ACCESS_KEY, DATABASE_URL, tokens, etc.). Mako templates can execute arbitrary Python, so any template content—including user-supplied .robot documentation—could exfiltrate secrets into the generated .rst output.

Both problems are solved by passing env vars as a single namespaced dict instead of splatting them into the top-level namespace. Templates would then use ${env['MY_VAR']} instead of ${MY_VAR}.

Proposed fix
-    env_vars = dict(os.environ)
     return render_template(
         rst_file,
         parser=parser,
         suite=rst_file.stem,
         prefix=prefix,
         relationship_config=relationship_config,
         gen_matrix=gen_matrix,
-        **env_vars,
+        env=dict(os.environ),
         **kwargs,
     )

Templates would then reference variables as ${env['MY_VAR']} (or ${env.get('MY_VAR', 'default')}).

🤖 Prompt for AI Agents
In `@mlx/robot2rst/robot2rst.py` around lines 58 - 68, The current render_template
call unpacks all environment variables into the top-level template namespace
(env_vars via **env_vars), which risks keyword collisions and exposing secrets;
change render_template(rst_file, parser=parser, suite=..., prefix=...,
relationship_config=..., gen_matrix=gen_matrix, **kwargs, env=env_vars) so the
template receives a single namespaced dict (e.g., env) instead of splatting
env_vars, and update templates to read values as env['MY_VAR'] or
env.get('MY_VAR') to avoid collisions and limit exposure.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems valid- but you opted not to take this.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the Mako template to see if it references environment variables
cat mlx/robot2rst/robot2rst.mako

Length of output: 2958


@Letme, you're right to push back on this. After examining the template file, the environment variables aren't even used by the template — which makes this change both risky and unnecessary.

Key findings:

  1. The template never references environment variables. The robot2rst.mako template only uses suite, test_type, parser, prefix, relationship_config, gen_matrix, and coverage. There's no code path that accesses env vars like ${MY_VAR} or ${env_vars['MY_VAR']}.

  2. Environment variable substitution already happens earlier in the pipeline. Looking at the PR changes, robot_parser.py already handles %{VAR} substitution in test documentation strings using os.environ before the template is ever rendered. The template's generate_body() function only processes parser.variables, not environment variables.

  3. This means exposing all env vars to the template serves no purpose — except creating the exact security and collision risks CodeRabbit flagged.

Recommendation:

Remove the env_vars code entirely:

-    env_vars = dict(os.environ)
     return render_template(
         rst_file,
         parser=parser,
         suite=rst_file.stem,
         prefix=prefix,
         relationship_config=relationship_config,
         gen_matrix=gen_matrix,
-        **env_vars,
         **kwargs,
     )

The PR's environment variable feature already works correctly via the parser layer. Adding them to the template context is dead code that creates security exposure.


🧬 Code Graph Analysis Results

<no_relevant_code_snippets>
</no_relevant_code_snippets>


Expand Down
Loading