Merge pull request #1168 from arnoldwender/fix/security-tunnels-permissions
fix(security): restrict tunnels.json file permissions
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
|
||||
a crash mid-write can never leave a partial/empty tunnels.json that
|
||||
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"
|
||||
with open(tmp_path, "w", encoding="utf-8") as f:
|
||||
json.dump(tunnels, f, indent=2)
|
||||
@@ -325,6 +337,10 @@ def _save_tunnels(tunnels):
|
||||
# Not all filesystems (or Windows file handles) support fsync — tolerate.
|
||||
pass
|
||||
os.replace(tmp_path, _TUNNEL_FILE)
|
||||
try:
|
||||
os.chmod(_TUNNEL_FILE, 0o600)
|
||||
except (OSError, NotImplementedError):
|
||||
pass
|
||||
|
||||
|
||||
def _endpoint_key(wing: str, room: str) -> str:
|
||||
|
||||
@@ -1,5 +1,8 @@
|
||||
"""Tests for explicit tunnel helpers in mempalace.palace_graph."""
|
||||
|
||||
import os
|
||||
import stat
|
||||
import sys
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
@@ -37,6 +40,33 @@ class TestTunnelStorage:
|
||||
palace_graph._save_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:
|
||||
def test_create_tunnel_deduplicates_reverse_order_and_updates_label(
|
||||
|
||||
Reference in New Issue
Block a user