Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
2 changes: 1 addition & 1 deletion meta_ads_mcp/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from meta_ads_mcp.core.server import main

__version__ = "1.0.126"
__version__ = "1.0.127"

__all__ = [
'get_ad_accounts',
Expand Down
48 changes: 45 additions & 3 deletions meta_ads_mcp/core/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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"

Expand All @@ -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)
Expand Down Expand Up @@ -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}")
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ build-backend = "hatchling.build"

[project]
name = "meta-ads-mcp"
version = "1.0.126"
version = "1.0.127"
description = "Model Context Protocol (MCP) server for Meta Ads - Use Remote MCP at pipeboard.co for easiest setup"
readme = "README.md"
requires-python = ">=3.10"
Expand Down
2 changes: 1 addition & 1 deletion server.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"$schema": "https://static.modelcontextprotocol.io/schemas/2025-12-11/server.schema.json",
"name": "co.pipeboard/meta-ads-mcp",
"description": "Facebook / Meta Ads automation with AI: analyze performance, test creatives, optimize spend.",
"version": "1.0.126",
"version": "1.0.127",
"remotes": [
{
"type": "streamable-http",
Expand Down
122 changes: 122 additions & 0 deletions tests/test_token_cache_permissions.py
Original file line number Diff line number Diff line change
@@ -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)
Loading