Skip to content

Commit fc95f66

Browse files
committed
refactor: deduplicate _to_boolean and fix TOCTOU in expansion cache
Extract duplicated _to_boolean logic from condition_resolver.py and fn_if.py into IntrinsicFunctionResolver.to_boolean() static method. Replace os.path.isfile() + os.path.getmtime() two-step check with a single try/except around getmtime() to eliminate the race condition.
1 parent ff95e81 commit fc95f66

4 files changed

Lines changed: 29 additions & 56 deletions

File tree

samcli/lib/cfn_language_extensions/resolvers/base.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,26 @@ def get_function_args(self, value: Dict[str, Any]) -> Any:
183183
"""
184184
return next(iter(value.values()))
185185

186+
@staticmethod
187+
def to_boolean(value: Any) -> bool:
188+
"""
189+
Convert a value to a boolean using CloudFormation semantics.
190+
191+
CloudFormation conditions can be:
192+
- Boolean values (True/False)
193+
- String "true"/"false" (case-insensitive)
194+
- Other types use Python's truthiness
195+
"""
196+
if isinstance(value, bool):
197+
return value
198+
if isinstance(value, str):
199+
lower_value = value.lower()
200+
if lower_value == "true":
201+
return True
202+
elif lower_value == "false":
203+
return False
204+
return bool(value)
205+
186206

187207
class IntrinsicResolver:
188208
"""

samcli/lib/cfn_language_extensions/resolvers/condition_resolver.py

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -356,32 +356,4 @@ def _resolve_nested(self, value: Any) -> Any:
356356
return value
357357

358358
def _to_boolean(self, value: Any) -> bool:
359-
"""
360-
Convert a value to a boolean.
361-
362-
CloudFormation conditions can be:
363-
- Boolean values (True/False)
364-
- String "true"/"false" (case-insensitive)
365-
366-
Args:
367-
value: The value to convert.
368-
369-
Returns:
370-
The boolean representation of the value.
371-
372-
Raises:
373-
InvalidTemplateException: If the value cannot be converted to boolean.
374-
"""
375-
if isinstance(value, bool):
376-
return value
377-
378-
if isinstance(value, str):
379-
lower_value = value.lower()
380-
if lower_value == "true":
381-
return True
382-
elif lower_value == "false":
383-
return False
384-
385-
# For other types, use Python's truthiness
386-
# This handles cases where nested intrinsics return non-boolean values
387-
return bool(value)
359+
return self.to_boolean(value)

samcli/lib/cfn_language_extensions/resolvers/fn_if.py

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -163,31 +163,7 @@ def _evaluate_condition(self, condition_name: str) -> bool:
163163
self.context._evaluating_conditions.discard(condition_name)
164164

165165
def _to_boolean(self, value: Any) -> bool:
166-
"""
167-
Convert a value to a boolean.
168-
169-
CloudFormation conditions can be:
170-
- Boolean values (True/False)
171-
- String "true"/"false" (case-insensitive)
172-
173-
Args:
174-
value: The value to convert.
175-
176-
Returns:
177-
The boolean representation of the value.
178-
"""
179-
if isinstance(value, bool):
180-
return value
181-
182-
if isinstance(value, str):
183-
lower_value = value.lower()
184-
if lower_value == "true":
185-
return True
186-
elif lower_value == "false":
187-
return False
188-
189-
# For other types, use Python's truthiness
190-
return bool(value)
166+
return self.to_boolean(value)
191167

192168
def _is_no_value_ref(self, value: Any) -> bool:
193169
"""

samcli/lib/cfn_language_extensions/sam_integration.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -534,8 +534,13 @@ def expand_language_extensions(
534534
"""
535535
# --- cache lookup ---
536536
cache_key: Optional[Tuple[str, float, str]] = None
537-
if template_path and os.path.isfile(template_path):
538-
cache_key = (template_path, os.path.getmtime(template_path), _hash_params(parameter_values))
537+
if template_path:
538+
try:
539+
mtime = os.path.getmtime(template_path)
540+
cache_key = (template_path, mtime, _hash_params(parameter_values))
541+
except OSError:
542+
pass
543+
if cache_key is not None:
539544
cached = _expansion_cache.get(cache_key)
540545
if cached is not None:
541546
LOG.debug("Cache hit for template expansion: %s", template_path)

0 commit comments

Comments
 (0)