fix(security): write the token cache owner-only (GHSA-prmg-4fr3-mm6x) (pipeboard-co#167)

The cache holding the long-lived Meta access token was created by a bare open(path, 'w') inside a directory made by mkdir() with no mode, so under the usual 0022 umask it landed at 0644 inside 0755 — readable by every other local user on a shared host.

The file is now created with os.open(..., O_CREAT, 0o600) and the directory with mode 0o700. O_CREAT's mode only applies at creation and mkdir's is masked by the umask, so both are narrowed explicitly afterwards; the same narrowing runs on load, which repairs a cache written by an earlier version without requiring re-authentication. The chmod step is skipped on Windows, where access is governed by ACLs.

Bumps to 1.0.127 and documents the advisory in SECURITY.md.

Author: nictuku

SECURITY.md

@@ -380,3 +380,39 @@ directory the server was started in, which is where the documented default
   matching `*_<hash>.jpg` outside your image directory.
 - Credited to [@Gal3m](https://github.com/Gal3m) and
   [@mohammad228](https://github.com/mohammad228).
+
+### GHSA-prmg-4fr3-mm6x — cached Meta access token was group/world-readable on disk
+
+- **Severity:** Medium
+- **Affected versions:** `<= 1.0.126`
+- **Fixed in:** `1.0.127`
+- **Affected configurations:** Local installs that authenticate through the
+  OAuth flow and cache a token (`~/.config/meta-ads-mcp/token_cache.json`, or
+  `~/Library/Application Support/meta-ads-mcp/` on macOS). Matters most on
+  shared hosts — CI runners, bastion boxes, multi-UID containers. The hosted MCP
+  at `*.mcp.pipeboard.co` does not cache tokens to disk.
+
+**What went wrong.** The cache file was created with a bare `open(path, "w")`
+and its directory with `mkdir()` with no mode, so under the usual `0022` umask
+the long-lived Meta access token (~60 days) landed in a `0644` file inside a
+`0755` directory. Any other local user could read it and act as the operator
+against the Graph API.
+
+**Fix.** The file is created with `os.open(..., O_CREAT, 0o600)` and the
+directory with mode `0o700`. Because `O_CREAT`'s mode applies only at creation
+and `mkdir`'s mode is masked by the umask (and is a no-op for a directory that
+already exists), both are also narrowed explicitly afterwards — which repairs a
+cache written by an earlier version. The same narrowing runs when a cache is
+loaded, so an existing install stops being readable as soon as the server
+starts, with no need to re-authenticate. On Windows the chmod step is skipped:
+it cannot express "owner only" there, where access is governed by ACLs.
+
+**Action for operators.**
+- Upgrade to `1.0.127` or later; the permissions are repaired on the next load
+  or save.
+- If you ran an earlier version on a shared host, treat the cached token as
+  exposed: rotate it at
+  `https://developers.facebook.com/tools/debug/accesstoken/` and review Graph
+  API access logs.
+- Credited to [@Gal3m](https://github.com/Gal3m) and
+  [@mohammad228](https://github.com/mohammad228).

meta_ads_mcp/core/auth.py

@@ -122,6 +122,30 @@ def deserialize(cls, data: Dict[str, Any]) -> 'TokenInfo':
         return token
 
 
+# The token cache holds a long-lived Meta access token (~60 days), so it must be
+# readable only by its owner. Before this, the file was created by a bare open()
+# and the directory by mkdir() with no mode, landing at 0644 inside 0755 under
+# the usual 0022 umask — readable by every local user. See GHSA-prmg-4fr3-mm6x.
+TOKEN_CACHE_FILE_MODE = 0o600
+TOKEN_CACHE_DIR_MODE = 0o700
+
+
+def _restrict_permissions(path: pathlib.Path, mode: int) -> None:
+    """Narrow `path` to `mode` if it is wider, best effort.
+
+    Also repairs caches written by earlier versions, so an operator does not
+    have to re-authenticate to get a private file. POSIX only: on Windows chmod
+    cannot express "owner only", and access is governed by ACLs instead.
+    """
+    if platform.system() == "Windows":
+        return
+    try:
+        if (path.stat().st_mode & 0o777) != mode:
+            os.chmod(path, mode)
+    except OSError as e:
+        logger.warning(f"Could not restrict permissions on {path}: {e}")
+
+
 class AuthManager:
     """Manages authentication with Meta APIs"""
     def __init__(self, app_id: str, redirect_uri: str = AUTH_REDIRECT_URI):
@@ -141,7 +165,10 @@ def _get_token_cache_path(self) -> pathlib.Path:
 
         # Create directory if it doesn't exist
         cache_dir = base_path / "meta-ads-mcp"
-        cache_dir.mkdir(parents=True, exist_ok=True)
+        cache_dir.mkdir(parents=True, exist_ok=True, mode=TOKEN_CACHE_DIR_MODE)
+        # mkdir's mode is masked by the umask and is a no-op for a directory that
+        # already exists, so narrow it explicitly either way.
+        _restrict_permissions(cache_dir, TOKEN_CACHE_DIR_MODE)
 
         return cache_dir / "token_cache.json"
 
@@ -152,6 +179,8 @@ def _load_cached_token(self) -> bool:
         if not cache_path.exists():
             return False
 
+        _restrict_permissions(cache_path, TOKEN_CACHE_FILE_MODE)
+        
         try:
             with open(cache_path, "r") as f:
                 data = json.load(f)
@@ -213,8 +242,21 @@ def _save_token_to_cache(self) -> None:
         cache_path = self._get_token_cache_path()
 
         try:
-            with open(cache_path, "w") as f:
-                json.dump(self.token_info.serialize(), f)
+            # O_CREAT's mode only applies when the file is created, so an
+            # existing cache from an earlier version is narrowed afterwards.
+            fd = os.open(
+                cache_path,
+                os.O_WRONLY | os.O_CREAT | os.O_TRUNC,
+                TOKEN_CACHE_FILE_MODE,
+            )
+            try:
+                cache_file = os.fdopen(fd, "w")
+            except Exception:
+                os.close(fd)
+                raise
+            with cache_file:
+                json.dump(self.token_info.serialize(), cache_file)
+            _restrict_permissions(cache_path, TOKEN_CACHE_FILE_MODE)
             logger.info(f"Token cached at: {cache_path}")
         except Exception as e:
             logger.error(f"Error saving token to cache: {e}")

tests/test_token_cache_permissions.py

@@ -0,0 +1,122 @@
+"""Regression tests for GHSA-prmg-4fr3-mm6x.
+
+The token cache holds a long-lived Meta access token (~60 days). It was written
+with a bare `open(path, "w")` into a directory created by `mkdir()` with no
+mode, so under the usual 0022 umask it landed at 0644 inside 0755 — readable by
+every other local user on a shared host.
+"""
+
+import json
+import os
+import platform
+import stat
+import time
+
+import pytest
+
+from meta_ads_mcp.core.auth import (
+    TOKEN_CACHE_DIR_MODE,
+    TOKEN_CACHE_FILE_MODE,
+    AuthManager,
+    TokenInfo,
+    _restrict_permissions,
+)
+
+pytestmark = pytest.mark.skipif(
+    platform.system() == "Windows",
+    reason="POSIX permission bits do not apply on Windows",
+)
+
+
+def _mode(path) -> int:
+    return stat.S_IMODE(os.stat(path).st_mode)
+
+
+@pytest.fixture
+def home(tmp_path, monkeypatch):
+    """Point the platform cache lookup at a scratch HOME."""
+    monkeypatch.setenv("HOME", str(tmp_path))
+    monkeypatch.setenv("APPDATA", str(tmp_path))
+    monkeypatch.setattr("pathlib.Path.home", lambda: tmp_path)
+    (tmp_path / "Library" / "Application Support").mkdir(parents=True, exist_ok=True)
+    (tmp_path / ".config").mkdir(parents=True, exist_ok=True)
+    return tmp_path
+
+
+@pytest.fixture
+def manager(home):
+    auth_manager = AuthManager(app_id="test-app-id")
+    auth_manager.token_info = TokenInfo(
+        access_token="EAAG" + "x" * 60,
+        expires_in=60 * 24 * 3600,
+        user_id="123",
+    )
+    return auth_manager
+
+
+def test_token_cache_file_is_owner_only(manager):
+    manager._save_token_to_cache()
+    cache_path = manager._get_token_cache_path()
+
+    assert _mode(cache_path) == TOKEN_CACHE_FILE_MODE
+    assert not _mode(cache_path) & 0o077, "group/other can read the Meta token"
+
+
+def test_token_cache_directory_is_owner_only(manager):
+    cache_dir = manager._get_token_cache_path().parent
+
+    assert _mode(cache_dir) == TOKEN_CACHE_DIR_MODE
+    assert not _mode(cache_dir) & 0o077
+
+
+def test_permissions_hold_under_a_permissive_umask(manager):
+    """0644 came from the umask, so the umask is what the fix must survive."""
+    previous = os.umask(0o000)
+    try:
+        manager._save_token_to_cache()
+    finally:
+        os.umask(previous)
+
+    assert _mode(manager._get_token_cache_path()) == TOKEN_CACHE_FILE_MODE
+
+
+def test_existing_world_readable_cache_is_narrowed_on_save(manager):
+    cache_path = manager._get_token_cache_path()
+    cache_path.write_text("{}")
+    os.chmod(cache_path, 0o644)
+
+    manager._save_token_to_cache()
+
+    assert _mode(cache_path) == TOKEN_CACHE_FILE_MODE
+
+
+def test_existing_world_readable_cache_is_narrowed_on_load(manager, home):
+    """A cache written by an older version is repaired without re-authenticating."""
+    cache_path = manager._get_token_cache_path()
+    cache_path.write_text(
+        json.dumps(
+            {
+                "access_token": "EAAG" + "y" * 60,
+                "expires_in": 60 * 24 * 3600,
+                "created_at": int(time.time()),
+                "user_id": "123",
+            }
+        )
+    )
+    os.chmod(cache_path, 0o644)
+
+    assert AuthManager(app_id="test-app-id")._load_cached_token() is True
+    assert _mode(cache_path) == TOKEN_CACHE_FILE_MODE
+
+
+def test_saved_token_is_still_readable_by_its_owner(manager):
+    manager._save_token_to_cache()
+
+    data = json.loads(manager._get_token_cache_path().read_text())
+
+    assert data["access_token"] == manager.token_info.access_token
+
+
+def test_restrict_permissions_survives_a_missing_file(tmp_path):
+    """Best effort: a vanished path must not raise into the caller."""
+    _restrict_permissions(tmp_path / "gone.json", TOKEN_CACHE_FILE_MODE)