You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@magpie.apache.org by po...@apache.org on 2026/10/18 18:28:44 UTC
(magpie) branch main updated: fix(local-tracker): keep new report files and folders to their owner (#1700)
This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/magpie.git
The following commit(s) were added to refs/heads/main by this push:
new 68ec4a33 fix(local-tracker): keep new report files and folders to their owner (#1700)
68ec4a33 is described below
commit 68ec4a339008bae7ca4e8c3ac2d7315912c2a06e
Author: Jarek Potiuk <ja...@potiuk.com>
AuthorDate: Sun Oct 18 20:28:37 2026 +0200
fix(local-tracker): keep new report files and folders to their owner (#1700)
* test(adversarial-review): give the answering reviewer time to start
test_one_timeout_does_not_block_the_others ran both stub reviewers with a
one-second budget, and the one meant to answer is a Python process that
has to start first. On a loaded machine (the parallel pre-commit pytest
run) start-up alone took longer, so the answering reviewer timed out too
and the test failed. Give it five seconds; the other stub still sleeps
for a minute, so the timeout case is unchanged.
Generated-by: Claude Opus 5
* test(adversarial-review): widen the other timing margins
Two more runner tests measured wall-clock time against bounds that left
little room for starting the Python stubs, and one failed under the
parallel pre-commit pytest run (3.03 s against 2.8 s):
- the parallel run sleeps 3 s per reviewer and must finish under 5.5 s
(6 s one after the other), not 1.5 s under 2.8 s;
- a finished answer with a lingering helper must arrive under 8 s with a
10 s timeout, not under 4 s with 5 s.
Each test still tells the behaviour apart from its failure mode.
Generated-by: Claude Opus 5
* fix(local-tracker): keep new report files and folders to their owner
The local tracker created report files with mode 0644 and their folders
with 0755, so under the usual umask any other user on the machine could
read an embargoed report and list the report folders, whose names carry
the title slug (CodeQL py/overly-permissive-file). Files and folders the
tracker creates are now 0600 and 0700; it never touches the mode of a
file that already exists.
Also drops an assignment in metadata._has_inline_comment that every
branch overwrote before reading it (CodeQL py/multiple-definition).
Generated-by: Claude Opus 5
---
.../local-tracker/src/magpie_local_tracker/metadata.py | 1 -
.../tools/local-tracker/src/magpie_local_tracker/store.py | 7 +++++--
tools/local-tracker/src/magpie_local_tracker/metadata.py | 1 -
tools/local-tracker/src/magpie_local_tracker/store.py | 7 +++++--
tools/local-tracker/tests/test_local_tracker.py | 14 ++++++++++++++
5 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/plugins/magpie-vetted-ops/tools/local-tracker/src/magpie_local_tracker/metadata.py b/plugins/magpie-vetted-ops/tools/local-tracker/src/magpie_local_tracker/metadata.py
index da7ab95c..8b19ce29 100644
--- a/plugins/magpie-vetted-ops/tools/local-tracker/src/magpie_local_tracker/metadata.py
+++ b/plugins/magpie-vetted-ops/tools/local-tracker/src/magpie_local_tracker/metadata.py
@@ -79,7 +79,6 @@ def _has_inline_comment(raw: str) -> bool:
scalar, a ``#`` preceded by whitespace starts a comment.
"""
text = raw.strip()
- rest = text
if text[:1] == "[":
# A flow sequence: quotes open quoted items at item boundaries only.
i, quote, at_item = 1, "", True
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 696bf342..e665ff2b 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
@@ -270,6 +270,9 @@ class Tracker:
A path-based check followed by `open()` leaves a window in which a
parent directory can be swapped for a symlink; opening each component
relative to the previous descriptor with `O_NOFOLLOW` closes it.
+
+ What it creates is the owner's alone (folders 0700, files 0600): the
+ reports are embargoed, and a folder's name carries the title slug.
"""
rel = os.path.relpath(os.path.abspath(path), os.path.abspath(self.root))
parts = Path(rel).parts
@@ -286,7 +289,7 @@ class Tracker:
raise
# Created concurrently is fine: open it below like any other.
with contextlib.suppress(FileExistsError):
- os.mkdir(name, 0o755, dir_fd=dir_fd)
+ os.mkdir(name, 0o700, dir_fd=dir_fd)
next_fd = os.open(name, dir_flags, dir_fd=dir_fd)
except OSError as exc:
if exc.errno in (errno.ELOOP, errno.ENOTDIR):
@@ -297,7 +300,7 @@ class Tracker:
os.close(dir_fd)
dir_fd = next_fd
try:
- return os.open(parts[-1], flags | os.O_NOFOLLOW, 0o644, dir_fd=dir_fd)
+ return os.open(parts[-1], flags | os.O_NOFOLLOW, 0o600, dir_fd=dir_fd)
except OSError as exc:
if exc.errno == errno.ELOOP:
raise refused(f"{path} is a symlink; refusing to follow it") from exc
diff --git a/tools/local-tracker/src/magpie_local_tracker/metadata.py b/tools/local-tracker/src/magpie_local_tracker/metadata.py
index da7ab95c..8b19ce29 100644
--- a/tools/local-tracker/src/magpie_local_tracker/metadata.py
+++ b/tools/local-tracker/src/magpie_local_tracker/metadata.py
@@ -79,7 +79,6 @@ def _has_inline_comment(raw: str) -> bool:
scalar, a ``#`` preceded by whitespace starts a comment.
"""
text = raw.strip()
- rest = text
if text[:1] == "[":
# A flow sequence: quotes open quoted items at item boundaries only.
i, quote, at_item = 1, "", True
diff --git a/tools/local-tracker/src/magpie_local_tracker/store.py b/tools/local-tracker/src/magpie_local_tracker/store.py
index 696bf342..e665ff2b 100644
--- a/tools/local-tracker/src/magpie_local_tracker/store.py
+++ b/tools/local-tracker/src/magpie_local_tracker/store.py
@@ -270,6 +270,9 @@ class Tracker:
A path-based check followed by `open()` leaves a window in which a
parent directory can be swapped for a symlink; opening each component
relative to the previous descriptor with `O_NOFOLLOW` closes it.
+
+ What it creates is the owner's alone (folders 0700, files 0600): the
+ reports are embargoed, and a folder's name carries the title slug.
"""
rel = os.path.relpath(os.path.abspath(path), os.path.abspath(self.root))
parts = Path(rel).parts
@@ -286,7 +289,7 @@ class Tracker:
raise
# Created concurrently is fine: open it below like any other.
with contextlib.suppress(FileExistsError):
- os.mkdir(name, 0o755, dir_fd=dir_fd)
+ os.mkdir(name, 0o700, dir_fd=dir_fd)
next_fd = os.open(name, dir_flags, dir_fd=dir_fd)
except OSError as exc:
if exc.errno in (errno.ELOOP, errno.ENOTDIR):
@@ -297,7 +300,7 @@ class Tracker:
os.close(dir_fd)
dir_fd = next_fd
try:
- return os.open(parts[-1], flags | os.O_NOFOLLOW, 0o644, dir_fd=dir_fd)
+ return os.open(parts[-1], flags | os.O_NOFOLLOW, 0o600, dir_fd=dir_fd)
except OSError as exc:
if exc.errno == errno.ELOOP:
raise refused(f"{path} is a symlink; refusing to follow it") from exc
diff --git a/tools/local-tracker/tests/test_local_tracker.py b/tools/local-tracker/tests/test_local_tracker.py
index 22fff70e..859c1bc5 100644
--- a/tools/local-tracker/tests/test_local_tracker.py
+++ b/tools/local-tracker/tests/test_local_tracker.py
@@ -4,6 +4,7 @@ from __future__ import annotations
import json
import shutil
+import stat
import subprocess
from pathlib import Path
from typing import Any
@@ -585,6 +586,19 @@ def test_unround_trippable_front_matter_is_never_rewritten(
assert (root / "VULN-1" / "report.md").read_text() == original
+def test_a_new_report_is_readable_by_its_owner_only(
+ fresh: Path, tmp_path: Path, capsys: pytest.CaptureFixture[str]
+) -> None:
+ """Reports are embargoed: neither the file nor its folder (named after the title) is open to other users."""
+ ident = new_report(capsys, fresh, tmp_path)
+ c = write(tmp_path / "c.md", "A comment")
+ assert run(capsys, fresh, "comment", ident, "--body-file", str(c), "--author", "@carol")[0] == 0
+ folder = fresh / ident
+ assert stat.S_IMODE(folder.stat().st_mode) == 0o700
+ written = list(folder.iterdir())
+ assert written and all(stat.S_IMODE(p.stat().st_mode) == 0o600 for p in written)
+
+
def test_create_retries_when_another_writer_took_the_id(
fresh: Path, tmp_path: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch
) -> None: