Skip to content

Jinja2 Sandbox Hardening - #953

Open
chrismaddalena wants to merge 5 commits into
masterfrom
hotfix/jinja2-sandbox
Open

Jinja2 Sandbox Hardening#953
chrismaddalena wants to merge 5 commits into
masterfrom
hotfix/jinja2-sandbox

Conversation

@chrismaddalena

Copy link
Copy Markdown
Collaborator

CHANGELOG

[7.2.5] - 31 July 2026

Security

  • Hardened Jinja2 report rendering against sandbox escapes while preserving user-authored report templates and previews
    • Operation-log values are treated as literal report data, including values containing captured Jinja2 payloads
    • Lazy rich-text rendering now rejects templates that were not compiled by Ghostwriter's sandboxed environment
    • Report template objects, Python callables, and document-export objects no longer expose unsafe attributes or call paths to Jinja2

Copilot AI review requested due to automatic review settings July 31, 2026 05:07
Comment thread ghostwriter/reporting/tests/test_rich_text_templating.py Fixed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Not ready to approve

The new JinjaReference TipTap node still serializes attacker-controlled reference strings into template source/attributes in a way that can reintroduce delimiter injection outside data-gw-jinja-literal wrappers.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR hardens Ghostwriter’s rich-text Jinja2 rendering pipeline to prevent sandbox escapes and delimiter-injection from stored assessment data (notably oplog values and evidence-friendly names), while preserving user-authored templating in report previews/exports.

Changes:

  • Introduces a ReportSandboxedEnvironment and tighter sandbox controls to block access to Jinja internals, unsafe attributes, and unregistered callables.
  • Adds a “literal fragment” mechanism for rich text (data-gw-jinja-literal) so captured/inserted payloads are excluded from Jinja compilation and restored only after rendering.
  • Updates oplog outline generation and evidence-name rewriting to avoid constructing unsafe legacy {{.ref ...}}/{{.Name}} source from untrusted values, with expanded regression tests.
File summaries
File Description
VERSION Updates release date for v7.2.5.
config/settings/base.py Updates RELEASE_DATE to match the release.
CHANGELOG.md Adds v7.2.5 security notes for Jinja2 sandbox hardening.
javascript/src/tiptap_gw/jinja_literal.ts Adds TipTap nodes for literal wrappers and structured reference markers.
javascript/src/tiptap_gw/index.ts Registers the new TipTap extensions.
javascript/src/frontend/collab_forms/rich_text_editor/oplog_outline.tsx Wraps inserted oplog-outline content as literal and inserts structured reference nodes.
ghostwriter/reporting/views2/report.py Emits outline blocks as literal data and introduces a structured reference block type.
ghostwriter/reporting/tests/test_views.py Updates outline expectations and adds preview hardening tests for literal content.
ghostwriter/reporting/tests/test_rich_text_templating.py Adds extensive sandbox hardening regression tests and literal-fragment tests.
ghostwriter/modules/reportwriter/project/base.py Treats oplog rich-text extra fields as literal report data during export context building.
ghostwriter/modules/reportwriter/base/html_rich_text.py Implements compiled rich-text templates with literal-fragment extraction/restoration and sandbox enforcement.
ghostwriter/modules/reportwriter/base/docx.py Prevents leaking exporter objects/attrs into templates and minor formatting adjustments.
ghostwriter/modules/reportwriter/base/base.py Adds process_literal_extra_fields() for non-templated rich-text sources (e.g., oplogs).
ghostwriter/modules/reportwriter/init.py Introduces ReportSandboxedEnvironment and jinja_string_literal(); updates env creation.
ghostwriter/api/views.py Rewrites evidence-friendly-name updates using safe literal encoding and mk_* calls.
ghostwriter/api/tests/test_views.py Adds tests ensuring evidence-name rewriting is delimiter-safe and rewrite-compatible.
Review details

Suppressed comments (1)

javascript/src/tiptap_gw/jinja_literal.ts:65

  • JinjaReference currently renders {{.ref ${node.attrs.ref}}} into both text and HTML output. If ref contains }}...{{ sequences, this recreates the exact delimiter-injection issue this PR is hardening against whenever the node is rendered outside a data-gw-jinja-literal container. Even when used safely today (wrapped for oplog outlines), users can copy/move the node and accidentally make it executable template source.

To keep references structured and non-executable, avoid emitting Jinja source here (no {{ ... }}) and rely on a safe marker representation plus an editor node view for display.

    renderText({ node }) {
        return `{{.ref ${node.attrs.ref}}}`;
    },

    renderHTML({ node, HTMLAttributes }) {
        return ["span", HTMLAttributes, `{{.ref ${node.attrs.ref}}}`];
    },
  • Files reviewed: 16/16 changed files
  • Comments generated: 1
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment thread javascript/src/tiptap_gw/jinja_literal.ts
@augmentcode

augmentcode Bot commented Jul 31, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: This PR hardens Ghostwriter’s report Jinja2 rendering to better resist sandbox escapes while keeping user-authored report templates and previews working.

Changes:

  • Introduced a custom ReportSandboxedEnvironment (immutable sandbox) and tightened safe attribute/callable rules to block access to Jinja internals and “capability” objects.
  • Added jinja_string_literal() and updated evidence friendly-name rewriting to use mk_evidence()/mk_ref() with encoded literal arguments (instead of legacy {{.name}}/{{.ref name}} construction).
  • Added rich-text literal wrappers (data-gw-jinja-literal) and a compile-time placeholder mechanism to keep marked HTML fragments out of Jinja parsing until after rendering.
  • Treated operation-log rich-text extra fields as literal report data (no Jinja compilation) to preserve captured payloads.
  • Updated oplog outline generation to return structured reference blocks and raw code output, with frontend insertion wrapped as literal data.
  • Added extensive Python test coverage for sandbox escape families, callable/attribute blocking, oplog literal handling, and preview behavior.
  • Bumped release date metadata for 7.2.5.

Technical Notes: The approach combines an immutable sandbox, capability-object blocking, and post-render restoration of marked literal HTML fragments to prevent normalization/escaping from turning stored data into executable Jinja source.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review completed. 2 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Comment thread ghostwriter/modules/reportwriter/__init__.py Outdated
Comment thread javascript/src/tiptap_gw/jinja_literal.ts Outdated
@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.77301% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.25%. Comparing base (392e271) to head (b558f57).

Files with missing lines Patch % Lines
...riter/reporting/tests/test_rich_text_templating.py 98.26% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #953      +/-   ##
==========================================
+ Coverage   92.19%   92.25%   +0.05%     
==========================================
  Files         444      444              
  Lines       33957    34235     +278     
==========================================
+ Hits        31308    31583     +275     
- Misses       2649     2652       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI review requested due to automatic review settings July 31, 2026 05:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Not ready to approve

The new “reference” TipTap node and literal-fragment extraction still allow user-controlled Jinja-like payloads to survive into template source in ways that can be executed or duplicated, undermining the sandbox-hardening goal.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (1)

javascript/src/tiptap_gw/jinja_literal.ts:65

  • JinjaReference currently serializes the reference as {{.ref ...}} text inside the <span data-gw-ref> node. That re-introduces user-controlled Jinja source (the ref value) into stored rich-text HTML, which can be parsed/executed during report rendering and can also produce duplicated ref expansion (nested data-gw-ref spans become Figure #Figure #). The span marker is sufficient for downstream expansion; the legacy {{.ref ...}} payload should not be emitted in stored HTML.
 */
export const JinjaReference = Node.create({
    name: "jinjaReference",
  • Files reviewed: 16/16 changed files
  • Comments generated: 1
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment thread ghostwriter/modules/reportwriter/base/html_rich_text.py
Copilot AI review requested due to automatic review settings July 31, 2026 05:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Human review recommended

It makes broad, security-critical changes to the Jinja2 sandbox and rich-text rendering behavior across backend and frontend, warranting final human review.

Review details

Suppressed comments (2)

ghostwriter/modules/reportwriter/base/html_rich_text.py:139

  • The rich_text_template() docstring still describes returning “a Jinja template”, but the function now returns a CompiledRichTextTemplate wrapper (and performs literal-fragment extraction/restoration). Updating the docstring will help callers understand the new contract and avoid assuming they can access jinja2.Template attributes on the return value.
def rich_text_template(
    env: jinja2.Environment,
    text: str,
) -> CompiledRichTextTemplate:
    """
    Converts rich text `text` to a Jinja template. This does some additional Ghostwriter-specific
    processing.
    """

javascript/src/tiptap_gw/jinja_literal.ts:29

  • referenceFromElement() always prefers data-gw-ref-encoded when present, even if decoding fails and returns an empty string. If a document ever contains both attributes (e.g., partially migrated content or user-edited HTML), an invalid/empty encoded value will clobber a valid legacy data-gw-ref value and the reference will be lost on parse. Prefer the decoded value only when it is non-empty; otherwise fall back to data-gw-ref.
function referenceFromElement(element: HTMLElement): string {
    const encodedRef = element.getAttribute(ENCODED_REFERENCE_ATTRIBUTE);
    if (encodedRef !== null) {
        return decodeReference(encodedRef);
    }
    return element.getAttribute("data-gw-ref") ?? "";
}
  • Files reviewed: 16/16 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Copilot AI review requested due to automatic review settings July 31, 2026 14:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Human review recommended

It introduces security-critical changes across the templating pipeline (frontend markers, backend sandboxing, and rich-text compilation), which warrants final human review despite strong test coverage.

Review details
  • Files reviewed: 16/16 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

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