fix(security): restrict tunnels.json file permissions
~/.mempalace/tunnels.json (introduced in #790) was created via plain open(..., "w") with no chmod, and its parent dir via os.makedirs() without mode=0o700. On Linux with default umask 022 both end up world-readable (0o644 / 0o755). Tunnels reveal cross-wing connections — which projects, people, and rooms the user has explicitly linked — so they are sensitive metadata that should not be readable by other local users on shared systems. Apply the same 0o700 / 0o600 pattern that #814 established for the other sensitive palace files. Chmod calls are wrapped in try/except (OSError, NotImplementedError) for Windows / unsupported-filesystem compatibility. Closes #1165
This commit is contained in:
@@ -313,8 +313,20 @@ def _save_tunnels(tunnels):
|
|||||||
Writes to ``tunnels.json.tmp`` then ``os.replace``s it into place, so
|
Writes to ``tunnels.json.tmp`` then ``os.replace``s it into place, so
|
||||||
a crash mid-write can never leave a partial/empty tunnels.json that
|
a crash mid-write can never leave a partial/empty tunnels.json that
|
||||||
silently wipes every tunnel on next read.
|
silently wipes every tunnel on next read.
|
||||||
|
|
||||||
|
Also restricts the parent directory to 0o700 and the file to 0o600 —
|
||||||
|
tunnels reveal cross-wing connections (which projects/people/rooms
|
||||||
|
the user has explicitly linked) and should not be world-readable on
|
||||||
|
shared Linux/multi-user systems. Matches the file-permission pattern
|
||||||
|
established by #814 for the other sensitive palace files.
|
||||||
"""
|
"""
|
||||||
os.makedirs(os.path.dirname(_TUNNEL_FILE), exist_ok=True)
|
parent = os.path.dirname(_TUNNEL_FILE)
|
||||||
|
os.makedirs(parent, exist_ok=True)
|
||||||
|
try:
|
||||||
|
os.chmod(parent, 0o700)
|
||||||
|
except (OSError, NotImplementedError):
|
||||||
|
# Windows / unsupported filesystems — tolerate.
|
||||||
|
pass
|
||||||
tmp_path = _TUNNEL_FILE + ".tmp"
|
tmp_path = _TUNNEL_FILE + ".tmp"
|
||||||
with open(tmp_path, "w", encoding="utf-8") as f:
|
with open(tmp_path, "w", encoding="utf-8") as f:
|
||||||
json.dump(tunnels, f, indent=2)
|
json.dump(tunnels, f, indent=2)
|
||||||
@@ -325,6 +337,10 @@ def _save_tunnels(tunnels):
|
|||||||
# Not all filesystems (or Windows file handles) support fsync — tolerate.
|
# Not all filesystems (or Windows file handles) support fsync — tolerate.
|
||||||
pass
|
pass
|
||||||
os.replace(tmp_path, _TUNNEL_FILE)
|
os.replace(tmp_path, _TUNNEL_FILE)
|
||||||
|
try:
|
||||||
|
os.chmod(_TUNNEL_FILE, 0o600)
|
||||||
|
except (OSError, NotImplementedError):
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
def _endpoint_key(wing: str, room: str) -> str:
|
def _endpoint_key(wing: str, room: str) -> str:
|
||||||
|
|||||||
@@ -1,5 +1,8 @@
|
|||||||
"""Tests for explicit tunnel helpers in mempalace.palace_graph."""
|
"""Tests for explicit tunnel helpers in mempalace.palace_graph."""
|
||||||
|
|
||||||
|
import os
|
||||||
|
import stat
|
||||||
|
import sys
|
||||||
from unittest.mock import MagicMock, patch
|
from unittest.mock import MagicMock, patch
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
@@ -37,6 +40,33 @@ class TestTunnelStorage:
|
|||||||
palace_graph._save_tunnels(tunnels)
|
palace_graph._save_tunnels(tunnels)
|
||||||
assert palace_graph._load_tunnels() == tunnels
|
assert palace_graph._load_tunnels() == tunnels
|
||||||
|
|
||||||
|
@pytest.mark.skipif(
|
||||||
|
sys.platform == "win32",
|
||||||
|
reason="POSIX file-permission bits only apply on Unix-like systems",
|
||||||
|
)
|
||||||
|
def test_save_tunnels_restricts_permissions(self, tmp_path, monkeypatch):
|
||||||
|
"""Regression for #1165 — tunnels.json reveals cross-wing links and
|
||||||
|
must not be world-readable on shared Linux/multi-user systems."""
|
||||||
|
tunnel_file = _use_tmp_tunnel_file(monkeypatch, tmp_path)
|
||||||
|
palace_graph._save_tunnels(
|
||||||
|
[
|
||||||
|
{
|
||||||
|
"id": "x",
|
||||||
|
"source": {"wing": "a", "room": "r1"},
|
||||||
|
"target": {"wing": "b", "room": "r2"},
|
||||||
|
"label": "",
|
||||||
|
}
|
||||||
|
]
|
||||||
|
)
|
||||||
|
|
||||||
|
file_mode = stat.S_IMODE(os.stat(tunnel_file).st_mode)
|
||||||
|
assert file_mode == 0o600, f"tunnels.json mode is {oct(file_mode)}, expected 0o600"
|
||||||
|
|
||||||
|
parent_mode = stat.S_IMODE(os.stat(tunnel_file.parent).st_mode)
|
||||||
|
assert (
|
||||||
|
parent_mode == 0o700
|
||||||
|
), f"tunnels.json parent dir mode is {oct(parent_mode)}, expected 0o700"
|
||||||
|
|
||||||
|
|
||||||
class TestExplicitTunnels:
|
class TestExplicitTunnels:
|
||||||
def test_create_tunnel_deduplicates_reverse_order_and_updates_label(
|
def test_create_tunnel_deduplicates_reverse_order_and_updates_label(
|
||||||
|
|||||||
Reference in New Issue
Block a user