Skip to content

Harden Jinja template rendering with sandboxing - #6888

Merged
blackboxsw merged 1 commit into
canonical:mainfrom
tcoatswo:fix/cloud-init-jinja-ssti-ready
Jul 15, 2026
Merged

Harden Jinja template rendering with sandboxing#6888
blackboxsw merged 1 commit into
canonical:mainfrom
tcoatswo:fix/cloud-init-jinja-ssti-ready

Conversation

@tcoatswo

Copy link
Copy Markdown
Contributor

Summary

Harden cloud-init Jinja template rendering by switching to a sandboxed Jinja environment.

Changes

  • replace direct jinja2.Template rendering with SandboxedEnvironment
  • handle SecurityError in the Jinja payload handler so unsafe templates are ignored cleanly
  • add unit regressions covering unsafe attribute access in templater and handler paths

Verification

  • reproduced the pre-fix behavior directly in Python: template expressions could access class, mro, and subclasses
  • verified post-fix behavior directly in Python: unsafe attribute access is blocked, and handler rendering returns None with a warning
  • I could not run the repo pytest suite on this host because the Python environment here does not have the pytest module installed

@blackboxsw blackboxsw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for this submission and increasing the security posture for cloud-init's jinja2 template handling. Integration tests look happy with this approach. Some things we'll need on this PR:

  • tox -e do_format

It looks like the sandboxedenvironment has been in Jinja2 for.... ever. So we don't have to worry about backward compatibility issues.

Requesting minor changes to ensure CI passes.

@blackboxsw blackboxsw self-assigned this Jun 12, 2026
@blackboxsw

Copy link
Copy Markdown
Collaborator

Let's resolve conflicts in this PR and get CI passing on this please.

Comment thread cloudinit/handlers/jinja_template.py Outdated
JUndefinedError: Type[Exception]
try:
from jinja2.exceptions import UndefinedError as JUndefinedError
from jinja2.exceptions import SecurityError as JSecurityError

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Note due to 473fa33 we are no longer allowing jinja2 to be an optional runtime dependency within try/except ImportError clauses. This is primary cause for the current merge conflicts.

@holmanb

holmanb commented Jul 14, 2026

Copy link
Copy Markdown
Member

@tcoatswo Any updates?

@holmanb holmanb added the stale-pr Pull request is stale; will be auto-closed soon label Jul 14, 2026
@tcoatswo

Copy link
Copy Markdown
Contributor Author

@tcoatswo Any updates?

Updated.

@holmanb
holmanb requested a review from blackboxsw July 14, 2026 21:52
@holmanb

holmanb commented Jul 14, 2026

Copy link
Copy Markdown
Member

@tcoatswo The CLA job is failing.

@tcoatswo
tcoatswo force-pushed the fix/cloud-init-jinja-ssti-ready branch from e83b234 to c4d1dee Compare July 14, 2026 21:57
@tcoatswo

Copy link
Copy Markdown
Contributor Author

Squashed the commits to resolve the CLA issue. Let me know if there's anything else!

@blackboxsw blackboxsw removed the stale-pr Pull request is stale; will be auto-closed soon label Jul 14, 2026
@blackboxsw

Copy link
Copy Markdown
Collaborator

@tcoatswo, changeset looks good thanks for resolving the merge conflicts. Our CLA service may not have received your CLA signing for the github username: tcoatswo. As the CLA service continues to report false for a github_username check which is what is rejecting this PR. Is it possible that the CLA was submitted under a different username?

curl https://cla.canonical.com/cla/check?github_usernames=tcoatswo
{"emails":{},"github_usernames":{"tcoatswo":false},"launchpad_usernames":{}}

@blackboxsw blackboxsw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Updated changes look good and conflicts resolved. Thanks @tcoatswo, just awaiting CI run.

I think we just need confirmation that the CLA was signed or an understanding about why CLA service is rejecting this github username.

@tcoatswo
tcoatswo force-pushed the fix/cloud-init-jinja-ssti-ready branch from c4d1dee to 9a35b52 Compare July 14, 2026 22:57
@tcoatswo

Copy link
Copy Markdown
Contributor Author

I've updated the commit author email to tyler@coatsworth.me which should be linked to the signed CLA. Since the commit email changed, it looks like the CI checks are currently requiring maintainer approval to run. Could you please approve the workflow run so we can verify if the CLA check passes now? Thanks!

@tcoatswo

Copy link
Copy Markdown
Contributor Author

Apologies for the back-and-forth! I realized I hadn't actually signed the CLA yet. I just created my account, signed the agreement, and linked my GitHub/email (tyler@coatsworth.me). The CLA check should pass now—could you please re-trigger the CI run when you have a moment? Thanks again for your patience!

@blackboxsw blackboxsw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you much @tcoatswo for the quick resolution here. CLA is happy and so are we.
let's ship it.

@blackboxsw
blackboxsw merged commit aad14e5 into canonical:main Jul 15, 2026
18 of 19 checks passed
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