Skip to content

Fix: allow __ prefixed temp variables in recipe validation#1111

Merged
jkasturi-sf merged 1 commit intomainfrom
fix/validation-dunder-temp-variables
Jan 9, 2026
Merged

Fix: allow __ prefixed temp variables in recipe validation#1111
jkasturi-sf merged 1 commit intomainfrom
fix/validation-dunder-temp-variables

Conversation

@aditya-balachander
Copy link
Copy Markdown
Contributor

Fix: Allow __ Prefixed Temp Variables in Recipe Validation

Problem

The recipe validation logic was incorrectly flagging temporary variables (those prefixed with __) as errors. Users would see validation errors like:

Jinja template error: '__gender_folder' is undefined
Jinja template error: Object has no attribute '__gender_folder'
access to attribute '__gender_folder' of 'MockObjectRow' object is unsafe

This blocked valid recipes that use __ prefixed variables as temporary/hidden fields from passing validation.

Root Causes

Three separate issues were identified through systematic debugging:

1. MockObjectRow.__getattr__ rejecting all underscore attributes

Location: recipe_validator.py MockObjectRow.__getattr__

The check if attr.startswith("_"): rejected ALL attributes starting with underscore, including valid __ prefixed temp variable fields that were properly defined in the object.

2. Jinja2 SandboxedEnvironment blocking __ attribute access

Location: recipe_validator.py SandboxedNativeEnvironment class

Jinja2's sandbox security blocks access to any attribute starting with underscore for security reasons. This is appropriate for preventing access to Python internals like __class__, but incorrectly blocked legitimate Snowfakery temp variables.

3. Errors propagating from context-mismatched field resolution

Location: recipe_validator.py MockObjectRow.__getattr__ field resolution

When resolving field values from a different object context (e.g., accessing tech_user.__gender_folder from within a ServiceResource object), errors would propagate instead of gracefully falling back to mock values.

Solution

Fix 1: Allow valid __ prefixed fields in MockObjectRow

# Before
if attr.startswith("_"):
    raise AttributeError(f"'{attr}' not found")

# After  
if attr.startswith("_") and attr not in self._all_field_names:
    raise AttributeError(f"'{attr}' not found")

Fix 2: Override is_safe_attribute in SandboxedNativeEnvironment

def is_safe_attribute(self, obj, attr, value):
    """Override to allow access to __ prefixed temp variable fields on MockObjectRow."""
    if attr.startswith("__") and "MockObjectRow" in type(obj).__name__:
        return True
    return super().is_safe_attribute(obj, attr, value)

Fix 3: Add error rollback for cross-context field resolution

When resolving field values fails due to context mismatch, remove the spurious errors and fall back to mock values instead of reporting false validation errors.

Testing

Added TestDoubleUnderscoreTempVariables test class with 9 comprehensive tests:

Test Description
test_dunder_variable_within_same_object Basic __ prefixed variable usage
test_dunder_variable_cross_object_reference Access via obj.__field syntax
test_multiple_dunder_variables Multiple __ vars working together
test_dunder_variable_in_nested_friends Nested friend object scenarios
test_dunder_variable_with_integer_operations Integer math operations
test_dunder_variable_conditional_logic Conditional expressions
test_dunder_variable_not_accessible_before_definition Forward reference errors
test_dunder_variable_in_random_choice_context Common random_choice pattern
test_dunder_variable_complex_nested_scenario Multi-level nesting

All 113 tests in test_recipe_validator.py pass.

Example Recipe That Now Validates

- object: User
  nickname: tech_user
  fields:
    __gender_folder: 
      random_choice: [men, women]
    FirstName: ${{ fake.FirstNameMale() if __gender_folder == 'men' else fake.FirstNameFemale() }}

- object: ServiceResource
  fields:
    ProfilePicture: "https://example.com/${{tech_user.__gender_folder}}/photo.jpg"

Backward Compatibility

This change is fully backward compatible. It only allows previously-rejected valid recipes to pass validation. No existing valid recipes are affected.

- Override is_safe_attribute in SandboxedNativeEnvironment to allow
  __ prefixed attributes on MockObjectRow objects
- Fix MockObjectRow.__getattr__ to allow valid __ prefixed fields
- Add error rollback for cross-context field resolution failures
- Add comprehensive test suite for __ prefixed temp variable handling
Copy link
Copy Markdown
Contributor

@jkasturi-sf jkasturi-sf left a comment

Choose a reason for hiding this comment

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

Looks good

@jkasturi-sf jkasturi-sf added this pull request to the merge queue Jan 9, 2026
Merged via the queue into main with commit 5e4731f Jan 9, 2026
14 of 16 checks passed
@jkasturi-sf jkasturi-sf deleted the fix/validation-dunder-temp-variables branch January 9, 2026 06:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants