Skip to content

[REVIEW] secure-code-review: add template sandbox and context-escaping evidence gates #1573

@wangedmund77-cmyk

Description

@wangedmund77-cmyk

Skill Being Reviewed

Skill name: secure-code-review
Skill path: skills/appsec/secure-code-review/SKILL.md

False Positive Analysis

Benign code/configuration that can be over-flagged:

return render_template("profile.html", bio=bleach.clean(user.bio))
# template autoescape remains enabled for HTML context

Why this is a false positive:
return render_template("profile.html", bio=bleach.clean(user.bio)) can be safe when the missing compensating evidence is present, but the current skill does not ask reviewers to distinguish that evidence from the risky pattern. The review should require proof of the guardrail before escalating severity.

Coverage Gaps

Missed variant 1:

template = env.from_string(request.json["template"])
return template.render(user=current_user, config=current_app.config)

Why it should be caught:
This variant leaves a realistic attack path open while still satisfying the current high-level checklist language.

Missed variant 2:

return render("email.html", { "cta": mark_safe(user_supplied_html) })
// safe in text preview, unsafe in HTML email body

Why it should be caught:
This is a common production edge case where policy exists on paper but does not bind the runtime behavior or evidence trail.

Edge Cases

Frameworks differ: Jinja, Twig, Django, Handlebars, Liquid, and ERB have different sandbox and escaping defaults. A generic XSS review can miss server-side template injection and context-specific escaping gaps.

Remediation Quality

  • Fix resolves the vulnerability
  • Fix doesn't introduce new security issues
  • Fix doesn't break functionality
  • Issues found: Add gates for user-controlled template source, sandbox mode, dangerous globals, autoescape status, and output context (HTML body, attribute, URL, JS string, email HTML).

Comparison to Other Tools

Tool Catches this? Notes
Semgrep Partial Has SSTI rules, but framework-specific and often misses dangerous globals.
CodeQL Partial Good for taint in supported languages but not every template engine.
Burp/ZAP Partial Can prove runtime SSTI/XSS when reachable, not code-only sandbox posture.

Overall Assessment

Strengths:
The skill is useful as a broad secure-code-review workflow with CWE mappings.

Needs improvement:
It should explicitly split XSS escaping from SSTI template-source control and sandbox evidence.

Priority recommendations:

  1. Add SSTI-specific review questions for template source, sandbox, globals, and loaders.
  2. Require context-specific escaping evidence rather than a single sanitized/unsanitized label.
  3. Include benign fixtures where autoescape and a restricted template set make rendering safe.

Bounty Info

  • I have read and agree to the CONTRIBUTING.md bounty terms
  • Preferred payment method: Crypto or PayPal after maintainer acceptance.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions