Skip to content

Commit 42c5883

Browse files
committed
Fix template autoescape, CAS single-return, and WebSocket SHA-1 hardening
Default Jinja rendering to autoescape=True and auto-detect HTML targets so user-supplied values can never inject raw markup. ContentStore.put / put_bytes now route through a single _write_atomic helper (single return point). The WebSocket handshake's RFC 6455 SHA-1 call passes usedforsecurity=False with a nosec tag since it is not a security primitive.
1 parent 803888d commit 42c5883

4 files changed

Lines changed: 97 additions & 26 deletions

File tree

automation_file/core/content_store.py

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
import hashlib
1212
import os
1313
import shutil
14-
from collections.abc import Iterator
14+
from collections.abc import Callable, Iterator
1515
from pathlib import Path
1616
from typing import IO
1717

@@ -48,36 +48,28 @@ def put(self, source: str | os.PathLike[str]) -> str:
4848
raise CASException(f"source is not a file: {src}")
4949
digest = self._hash_file(src)
5050
target = self.path_for(digest)
51-
if target.exists():
52-
return digest
53-
target.parent.mkdir(parents=True, exist_ok=True)
54-
tmp = target.with_suffix(target.suffix + ".tmp")
55-
try:
56-
shutil.copyfile(src, tmp)
57-
os.replace(tmp, target)
58-
except OSError as error:
59-
if tmp.exists():
60-
tmp.unlink(missing_ok=True)
61-
raise CASException(f"failed to ingest {src}: {error}") from error
51+
if not target.exists():
52+
self._write_atomic(target, lambda tmp: _copyfile(src, tmp))
6253
return digest
6354

6455
def put_bytes(self, data: bytes) -> str:
6556
"""Ingest raw bytes and return the hex digest."""
6657
digest = hashlib.new(_HASH, data).hexdigest()
6758
target = self.path_for(digest)
68-
if target.exists():
69-
return digest
59+
if not target.exists():
60+
self._write_atomic(target, lambda tmp: _write_bytes(tmp, data))
61+
return digest
62+
63+
def _write_atomic(self, target: Path, writer: Callable[[Path], None]) -> None:
7064
target.parent.mkdir(parents=True, exist_ok=True)
7165
tmp = target.with_suffix(target.suffix + ".tmp")
7266
try:
73-
with open(tmp, "wb") as fh:
74-
fh.write(data)
67+
writer(tmp)
7568
os.replace(tmp, target)
7669
except OSError as error:
7770
if tmp.exists():
7871
tmp.unlink(missing_ok=True)
7972
raise CASException(f"failed to store blob: {error}") from error
80-
return digest
8173

8274
def open(self, digest: str) -> IO[bytes]:
8375
"""Open the stored blob for binary read."""
@@ -125,3 +117,12 @@ def _hash_file(self, path: Path) -> str:
125117
for chunk in iter(lambda: fh.read(_CHUNK), b""):
126118
hasher.update(chunk)
127119
return hasher.hexdigest()
120+
121+
122+
def _write_bytes(target: Path, data: bytes) -> None:
123+
with open(target, "wb") as fh:
124+
fh.write(data)
125+
126+
127+
def _copyfile(src: Path, dst: Path) -> None:
128+
shutil.copyfile(src, dst)

automation_file/local/templates.py

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,21 @@
1515
from automation_file.exceptions import TemplateException
1616

1717

18-
def render_string(template: str, context: dict[str, Any], *, use_jinja: bool = True) -> str:
19-
"""Render ``template`` against ``context`` and return the string result."""
18+
def render_string(
19+
template: str,
20+
context: dict[str, Any],
21+
*,
22+
use_jinja: bool = True,
23+
autoescape: bool = True,
24+
) -> str:
25+
"""Render ``template`` against ``context`` and return the string result.
26+
27+
``autoescape=True`` (the default) HTML-escapes substituted values when the
28+
Jinja2 engine is used; pass ``autoescape=False`` only when the output is
29+
known to target a non-HTML format.
30+
"""
2031
if use_jinja:
21-
rendered = _render_with_jinja(template, context)
32+
rendered = _render_with_jinja(template, context, autoescape=autoescape)
2233
if rendered is not None:
2334
return rendered
2435
return _render_with_stdlib(template, context)
@@ -30,11 +41,15 @@ def render_file(
3041
output_path: str | os.PathLike[str] | None = None,
3142
*,
3243
use_jinja: bool | None = None,
44+
autoescape: bool | None = None,
3345
) -> str:
3446
"""Read a template file, render it, optionally write the result.
3547
3648
``use_jinja=None`` auto-detects: ``.j2`` / ``.jinja`` / ``.jinja2`` suffixes
3749
opt in to Jinja2; other extensions use :class:`string.Template`.
50+
51+
``autoescape=None`` auto-detects based on the output path suffix — HTML /
52+
XML targets enable escaping, others disable it. Pass a bool to override.
3853
"""
3954
src = Path(template_path)
4055
if not src.is_file():
@@ -44,25 +59,45 @@ def render_file(
4459
except OSError as error:
4560
raise TemplateException(f"cannot read template {src}: {error}") from error
4661
jinja = _wants_jinja(src) if use_jinja is None else use_jinja
47-
rendered = render_string(source, context, use_jinja=jinja)
62+
escape = _wants_autoescape(output_path, src) if autoescape is None else autoescape
63+
rendered = render_string(source, context, use_jinja=jinja, autoescape=escape)
4864
if output_path is not None:
4965
dest = Path(output_path)
5066
dest.parent.mkdir(parents=True, exist_ok=True)
5167
dest.write_text(rendered, encoding="utf-8")
5268
return rendered
5369

5470

71+
_HTML_SUFFIXES = frozenset({".html", ".htm", ".xhtml", ".xml"})
72+
73+
5574
def _wants_jinja(path: Path) -> bool:
5675
return path.suffix.lower() in {".j2", ".jinja", ".jinja2"}
5776

5877

59-
def _render_with_jinja(template: str, context: dict[str, Any]) -> str | None:
78+
def _wants_autoescape(
79+
output_path: str | os.PathLike[str] | None,
80+
template_path: Path,
81+
) -> bool:
82+
target = Path(output_path) if output_path is not None else template_path
83+
suffix = target.suffix.lower()
84+
if suffix in {".j2", ".jinja", ".jinja2"}:
85+
suffix = target.with_suffix("").suffix.lower()
86+
return suffix in _HTML_SUFFIXES
87+
88+
89+
def _render_with_jinja(
90+
template: str,
91+
context: dict[str, Any],
92+
*,
93+
autoescape: bool,
94+
) -> str | None:
6095
try:
6196
from jinja2 import Environment, StrictUndefined
6297
from jinja2 import TemplateError as JinjaTemplateError
6398
except ImportError:
6499
return None
65-
env = Environment(autoescape=False, undefined=StrictUndefined)
100+
env = Environment(autoescape=autoescape, undefined=StrictUndefined)
66101
try:
67102
return env.from_string(template).render(**context)
68103
except JinjaTemplateError as error:

automation_file/server/_websocket.py

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,18 @@
2020

2121

2222
def compute_accept_key(sec_websocket_key: str) -> str:
23-
"""Return the ``Sec-WebSocket-Accept`` value for ``sec_websocket_key``."""
24-
digest = hashlib.sha1((sec_websocket_key + _GUID).encode("ascii")).digest()
23+
"""Return the ``Sec-WebSocket-Accept`` value for ``sec_websocket_key``.
24+
25+
RFC 6455 mandates SHA-1 + a fixed GUID for the opening handshake — the
26+
digest only proves the server understood the handshake, it is not a
27+
security primitive. ``usedforsecurity=False`` tells static analysers to
28+
skip the standard SHA-1 "insecure hash" warning.
29+
"""
30+
# nosec B303 B324 - RFC 6455 handshake, not a security primitive.
31+
digest = hashlib.sha1(
32+
(sec_websocket_key + _GUID).encode("ascii"),
33+
usedforsecurity=False,
34+
).digest()
2535
return base64.b64encode(digest).decode("ascii")
2636

2737

tests/test_templates.py

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
def _has_jinja() -> bool:
1414
try:
15-
import jinja2 # noqa: F401
15+
import jinja2 # noqa: F401 # pylint: disable=import-outside-toplevel,unused-import
1616
except ImportError:
1717
return False
1818
return True
@@ -64,3 +64,28 @@ def test_render_file_auto_detects_jinja_by_suffix(tmp_path: Path) -> None:
6464
tmpl = tmp_path / "page.j2"
6565
tmpl.write_text("{% for v in values %}{{ v }};{% endfor %}", encoding="utf-8")
6666
assert render_file(tmpl, {"values": [1, 2, 3]}) == "1;2;3;"
67+
68+
69+
@pytest.mark.skipif(not _has_jinja(), reason="jinja2 not installed")
70+
def test_render_string_jinja_autoescapes_html_by_default() -> None:
71+
assert render_string("{{ x }}", {"x": "<script>"}) == "&lt;script&gt;"
72+
73+
74+
@pytest.mark.skipif(not _has_jinja(), reason="jinja2 not installed")
75+
def test_render_string_jinja_autoescape_opt_out() -> None:
76+
assert render_string("{{ x }}", {"x": "<script>"}, autoescape=False) == "<script>"
77+
78+
79+
@pytest.mark.skipif(not _has_jinja(), reason="jinja2 not installed")
80+
def test_render_file_autoescapes_when_output_is_html(tmp_path: Path) -> None:
81+
tmpl = tmp_path / "page.html.j2"
82+
tmpl.write_text("{{ x }}", encoding="utf-8")
83+
out = tmp_path / "page.html"
84+
assert render_file(tmpl, {"x": "<b>"}, out) == "&lt;b&gt;"
85+
86+
87+
@pytest.mark.skipif(not _has_jinja(), reason="jinja2 not installed")
88+
def test_render_file_skips_autoescape_for_non_html(tmp_path: Path) -> None:
89+
tmpl = tmp_path / "note.txt"
90+
tmpl.write_text("{{ x }}", encoding="utf-8")
91+
assert render_file(tmpl, {"x": "<b>"}, use_jinja=True) == "<b>"

0 commit comments

Comments
 (0)