(magpie) 06/08: fix(local-tracker): report text cannot mix a file's line endings

[po...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

potiuk pushed a commit to branch feat/body-field-contract
in repository https://gitbox.apache.org/repos/asf/magpie.git

commit d711342092ffafaefff9398e187df8116c3073e9
Author: Jarek Potiuk <po...@apache.org>
AuthorDate: Sun Oct 18 17:13:48 2026 +0200

    fix(local-tracker): report text cannot mix a file's line endings
    
    A write keeps the file's own line-ending convention and refuses a file
    that mixes several. Report text comes from outside the team, though, and
    a report or field value carrying its own \r\n or lone \r was written as
    it came: the file then mixed conventions, and every later write to that
    tracker was refused until someone fixed the file by hand.
    
    Untrusted text now has its line endings turned into \n before it is
    escaped, and every write normalises its text before putting the file's
    convention back, so no write can leave a file mixing them. A file that
    already mixed them before the tracker touched it is still refused.
    
    Generated-by: Claude Opus 5
---
 .../src/magpie_local_tracker/sections.py           |  6 +++++
 .../src/magpie_local_tracker/store.py              |  5 +++-
 .../src/magpie_local_tracker/sections.py           |  6 +++++
 .../src/magpie_local_tracker/store.py              |  5 +++-
 .../tests/test_storage_conformance.py              | 31 ++++++++++++++++++++++
 5 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/plugins/magpie-vetted-ops/tools/local-tracker/src/magpie_local_tracker/sections.py b/plugins/magpie-vetted-ops/tools/local-tracker/src/magpie_local_tracker/sections.py
index d0803210..8f71c002 100644
--- a/plugins/magpie-vetted-ops/tools/local-tracker/src/magpie_local_tracker/sections.py
+++ b/plugins/magpie-vetted-ops/tools/local-tracker/src/magpie_local_tracker/sections.py
@@ -167,7 +167,13 @@ def neutralize(text: str, managed_headings: tuple[str, ...] = (), *, body: bool
     fields: those stay, but every one inside a closed code block is escaped
     (:func:`escape_quoted_fields`), and a fence left open is closed in the
     field it opens in, never past another field's heading.
+
+    Line endings become ``\n`` first: the file is read back with ``\r\n`` and
+    a lone ``\r`` both ending a line, so a ``\r`` left in would start a line
+    the escaping never saw, and a stray ``\r\n`` would leave the file mixing
+    conventions, which every later write refuses.
     """
+    text = text.replace("\r\n", "\n").replace("\r", "\n")
     text = _FORGEABLE.sub("&lt;", text)
     if body:
         text = close_fences_in_fields(escape_quoted_fields(text))
diff --git a/plugins/magpie-vetted-ops/tools/local-tracker/src/magpie_local_tracker/store.py b/plugins/magpie-vetted-ops/tools/local-tracker/src/magpie_local_tracker/store.py
index 5eb4a9c2..696bf342 100644
--- a/plugins/magpie-vetted-ops/tools/local-tracker/src/magpie_local_tracker/store.py
+++ b/plugins/magpie-vetted-ops/tools/local-tracker/src/magpie_local_tracker/store.py
@@ -365,6 +365,9 @@ class Tracker:
             raise refused(
                 f"{path} mixes line endings; normalise them to one convention before the tracker writes to it"
             )
+        # Whatever reached here is written in the file's one convention, so a
+        # write never turns a file into one that mixes line endings.
+        text = text.replace("\r\n", "\n").replace("\r", "\n")
         if not final_newline and text.endswith("\n"):
             text = text[:-1]
         if eol != "\n":
@@ -621,7 +624,7 @@ class Tracker:
         text = self._clean(body, body=True).strip("\n") + "\n"
         if self.desc.title_source == "h1":
             text = f"# {title}\n\n{text}"
-        rendered = md.render(meta, text)
+        rendered = md.render(meta, text).replace("\r\n", "\n").replace("\r", "\n")
         # Reserve the ID with an exclusive create: two writers that picked the
         # same number cannot both succeed, and the loser takes the next one.
         for _attempt in range(50):
diff --git a/tools/local-tracker/src/magpie_local_tracker/sections.py b/tools/local-tracker/src/magpie_local_tracker/sections.py
index d0803210..8f71c002 100644
--- a/tools/local-tracker/src/magpie_local_tracker/sections.py
+++ b/tools/local-tracker/src/magpie_local_tracker/sections.py
@@ -167,7 +167,13 @@ def neutralize(text: str, managed_headings: tuple[str, ...] = (), *, body: bool
     fields: those stay, but every one inside a closed code block is escaped
     (:func:`escape_quoted_fields`), and a fence left open is closed in the
     field it opens in, never past another field's heading.
+
+    Line endings become ``\n`` first: the file is read back with ``\r\n`` and
+    a lone ``\r`` both ending a line, so a ``\r`` left in would start a line
+    the escaping never saw, and a stray ``\r\n`` would leave the file mixing
+    conventions, which every later write refuses.
     """
+    text = text.replace("\r\n", "\n").replace("\r", "\n")
     text = _FORGEABLE.sub("&lt;", text)
     if body:
         text = close_fences_in_fields(escape_quoted_fields(text))
diff --git a/tools/local-tracker/src/magpie_local_tracker/store.py b/tools/local-tracker/src/magpie_local_tracker/store.py
index 5eb4a9c2..696bf342 100644
--- a/tools/local-tracker/src/magpie_local_tracker/store.py
+++ b/tools/local-tracker/src/magpie_local_tracker/store.py
@@ -365,6 +365,9 @@ class Tracker:
             raise refused(
                 f"{path} mixes line endings; normalise them to one convention before the tracker writes to it"
             )
+        # Whatever reached here is written in the file's one convention, so a
+        # write never turns a file into one that mixes line endings.
+        text = text.replace("\r\n", "\n").replace("\r", "\n")
         if not final_newline and text.endswith("\n"):
             text = text[:-1]
         if eol != "\n":
@@ -621,7 +624,7 @@ class Tracker:
         text = self._clean(body, body=True).strip("\n") + "\n"
         if self.desc.title_source == "h1":
             text = f"# {title}\n\n{text}"
-        rendered = md.render(meta, text)
+        rendered = md.render(meta, text).replace("\r\n", "\n").replace("\r", "\n")
         # Reserve the ID with an exclusive create: two writers that picked the
         # same number cannot both succeed, and the loser takes the next one.
         for _attempt in range(50):
diff --git a/tools/local-tracker/tests/test_storage_conformance.py b/tools/local-tracker/tests/test_storage_conformance.py
index 37590efd..94264ca4 100644
--- a/tools/local-tracker/tests/test_storage_conformance.py
+++ b/tools/local-tracker/tests/test_storage_conformance.py
@@ -298,3 +298,34 @@ def test_a_file_mixing_line_endings_is_refused(tmp_path: Path) -> None:
         tracker.field_set("SEC-0001", "Severity", "High")
     assert err.value.code == 4 and "mixes line endings" in str(err.value)
     assert path.read_bytes() == raw
+
+
+def test_carriage_returns_in_report_text_cannot_lock_the_file(tmp_path: Path) -> None:
+    """Report text is untrusted: its own ``\\r`` must neither mix the file's
+    line endings (which would refuse every later write) nor start a line the
+    escaping never saw."""
+    from magpie_local_tracker import main
+
+    root = tmp_path / "security"
+    root.mkdir()
+    assert main(["--root", str(root), "init", "--access-restricted"]) == 0
+    tracker = Tracker(root, load(root))
+    body = "### Severity\r\n\r\nLow\r\n\r\n### Note\r\n\r\nseen\rtwice\n"
+    created = tracker.create("A report", body, [])
+    ident, path = str(created["id"]), Path(str(created["path"]))
+    assert b"\r" not in path.read_bytes()
+    tracker.field_set(ident, "Severity", "High\r\nor\rmore\r### Forged\r\rno")
+    tracker.comment(ident, "a comment\r\nover\rlines", "@lead")
+    tracker.comment(ident, "a second one", "@lead")  # the comments file is still writable
+    for written in path.parent.iterdir():
+        assert b"\r" not in written.read_bytes(), written.name
+    assert str(tracker.field_get(ident, "Severity")["value"]).startswith("High\nor\nmore\n")
+    assert tracker.field_get(ident, "Forged")["value"] is None
+
+
+def test_a_carriage_return_in_a_value_keeps_the_files_one_convention(tmp_path: Path) -> None:
+    tracker, path = _plain(tmp_path, (FRONT + "### Severity\n\nLow\n").replace("\n", "\r\n").encode())
+    tracker.field_set("SEC-0001", "Severity", "High\rstill")
+    assert not set(path.read_bytes().decode().replace("\r\n", "")) & {"\r", "\n"}
+    tracker.field_set("SEC-0001", "Severity", "Critical")  # still writable
+    assert tracker.field_get("SEC-0001", "Severity")["value"] == "Critical"