Skip to content

fix: avoid unsafe yaml load in conformance tests - #668

Open
hln33 wants to merge 1 commit into
mainfrom
fix/unsafe-yaml-load
Open

fix: avoid unsafe yaml load in conformance tests#668
hln33 wants to merge 1 commit into
mainfrom
fix/unsafe-yaml-load

Conversation

@hln33

@hln33 hln33 commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Issue #, if available:
N/A

Description of changes:

  • Replace yaml.load() occurrence with a helper function _safe_load_cfn
  • yaml.load() is flagged as a security risk
    • In this case, it is a false positive as the flagged code was yaml.load(stream, Loader=CfnLoader) and CfnLoader extends yaml.SafeLoader
  • _safe_load_cfn is the equivalent of yaml.load(stream, Loader=CfnLoader) except it does not flag as a security risk
  • The same issue and fix was done for the JS SDK - fix(testing-sdk): avoid yaml.load call form flagged by CodeQL aws-durable-execution-sdk-js#750

Testing:

  • Ran hatch run test:all packages/aws-durable-execution-sdk-python-conformance-tests-otel/tests/test_otel_examples.py and all tests passed

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@hln33
hln33 deployed to ai-pr-review-runtime August 20, 2026 23:43 — with GitHub Actions Active
@hln33
hln33 deployed to ai-pr-review-runtime August 20, 2026 23:43 — with GitHub Actions Active
@github-actions

Copy link
Copy Markdown
Contributor

Codex AI review

No actionable findings. Residual risk is low: existing tests exercise both CloudFormation templates through the new loader helper, though tests were not run per review constraints.

Reviewed commit 13b9a1a5de5de773a0abdb1441bfc8858e7572f1. Workflow run

@github-actions

Copy link
Copy Markdown
Contributor

Claude AI review

The PR replaces a CodeQL-flagged yaml.load(stream, Loader=CfnLoader) call with a _safe_load_cfn helper and documents the rule in AGENTS.md. The helper (CfnLoader(stream)get_single_data()dispose() in a finally) is a faithful reproduction of PyYAML's internal yaml.load implementation, so behavior is unchanged while removing the unsafe call form. CfnLoader subclasses yaml.SafeLoader and registers its short-form tag constructor at the class level, so direct instantiation preserves both safe-loading and CloudFormation tag support. The Any return type is compatible with the caller's dict[str, Any] annotation, and empty-stream handling matches the prior path.

No findings. The change is correct and behavior-preserving. Residual test risk is low: no new test is added, but the existing template/long_running_template fixtures already exercise load_template, so the refactored code path stays covered, which is appropriate for an isolated fix under the AGENTS.md testing rules.

Reviewed commit 13b9a1a5de5de773a0abdb1441bfc8858e7572f1. Workflow run

@yaythomas

Copy link
Copy Markdown
Contributor

tracking otel test failure #669

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.

3 participants