fix(security): stop writing access tokens to the debug log (GHSA-r3r9-3mrh-x966)

setup_logging called logging.basicConfig(level=DEBUG, filename=...), which installs a handler on the ROOT logger, so third-party records were captured too — including httpx's INFO line, which prints the full request URL. The Graph API takes access_token as a query parameter, so every API call wrote the operator's token in full into a 0644 file that is never rotated. The package's own request logging masked it; httpx's did not.

The handler now belongs to this package's logger with propagate=False, httpx and httpcore are held at WARNING so the line is not emitted into host-configured logging either, the default level is INFO (META_ADS_LOG_LEVEL=DEBUG restores it), and the log file is 0600 inside a 0700 directory, narrowed on startup if an earlier version left them wider.

Credential prefixes (token[:10]) in log lines and in the get_login_link response are replaced by redact_secret(), which reports the length only.

The pipeboard_auth.py query-string half of the report no longer applies: that module was removed in 1.0.121 and no credential is placed in a URL by this package today.

Bumps to 1.0.128 and documents the advisory in SECURITY.md, including deleting existing logs and rotating the token.

Author: nictuku

SECURITY.md

@@ -416,3 +416,55 @@ it cannot express "owner only" there, where access is governed by ACLs.
   API access logs.
 - Credited to [@Gal3m](https://github.com/Gal3m) and
   [@mohammad228](https://github.com/mohammad228).
+
+### GHSA-r3r9-3mrh-x966 — access token written to the debug log via third-party request logging
+
+- **Severity:** Medium
+- **Affected versions:** `<= 1.0.127`
+- **Fixed in:** `1.0.128`
+- **Affected configurations:** Any install that makes Graph API calls. The log
+  file lives at `~/.config/meta-ads-mcp/meta_ads_debug.log`
+  (`~/Library/Application Support/meta-ads-mcp/` on macOS) and is never rotated,
+  so records accumulate indefinitely. It matters most where logs are shipped
+  somewhere else — container stdout collection, a log aggregator, a SIEM — or on
+  a shared host.
+
+**What the report described, and what was still live.** The advisory cites
+`pipeboard_auth.py`, which put `PIPEBOARD_API_TOKEN` in a URL query string and
+logged the full URL. That module was removed in `1.0.121`, and no credential is
+placed in a query string by this package today, so that part no longer applies.
+
+The logging half did still apply, through a different route. `setup_logging()`
+called `logging.basicConfig(level=DEBUG, filename=...)`, which installs a handler
+on the **root** logger — so every third-party record was captured too, including
+httpx's INFO line:
+
+```
+httpx - INFO - HTTP Request: GET https://graph.facebook.com/v24.0/me/adaccounts?access_token=<the operator's token> "HTTP/1.1 200 OK"
+```
+
+The package masks the token in its own request logging, but the Graph API takes
+`access_token` as a query parameter, so httpx logged it in full on every call.
+The file was created at `0644`.
+
+**Fix.**
+1. The file handler is attached to this package's logger with `propagate = False`
+   instead of the root logger, so third-party records are no longer captured.
+2. `httpx` and `httpcore` are held at `WARNING`, so the URL line is not emitted
+   into logging the host application configures either.
+3. The default level is `INFO` rather than `DEBUG`; set `META_ADS_LOG_LEVEL=DEBUG`
+   to restore the old verbosity.
+4. The log file is created `0600` inside a `0700` directory, and both are
+   narrowed on startup if an earlier version left them wider.
+5. Credential prefixes (`token[:10]`) in log lines and in the `get_login_link`
+   response are replaced by `redact_secret()`, which reports the length only. Ten
+   characters is still credential material in a file that is never rotated, and
+   enough to correlate one caller's requests on a shared deployment.
+
+**Action for operators.**
+- Upgrade to `1.0.128` or later.
+- Existing `meta_ads_debug.log` files contain access tokens in full. Delete them,
+  and if the logs were shipped anywhere or the host is shared, rotate the token
+  at `https://developers.facebook.com/tools/debug/accesstoken/`.
+- Credited to [@Gal3m](https://github.com/Gal3m) and
+  [@mohammad228](https://github.com/mohammad228).

meta_ads_mcp/__init__.py

@@ -6,7 +6,7 @@
 
 from meta_ads_mcp.core.server import main
 
-__version__ = "1.0.127"
+__version__ = "1.0.128"
 
 __all__ = [
     'get_ad_accounts',

meta_ads_mcp/core/auth.py

@@ -8,7 +8,7 @@
 import webbrowser
 import asyncio
 import json
-from .utils import logger
+from .utils import logger, redact_secret, restrict_permissions
 import requests
 
 # Import from the new callback server module
@@ -130,20 +130,9 @@ def deserialize(cls, data: Dict[str, Any]) -> 'TokenInfo':
 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}")
+# Shared with the log file in utils; repairs caches written by earlier versions
+# so an operator does not have to re-authenticate to get a private file.
+_restrict_permissions = restrict_permissions
 
 
 class AuthManager:
@@ -329,7 +318,7 @@ def get_access_token(self) -> Optional[str]:
     def invalidate_token(self) -> None:
         """Invalidate the current token, usually because it has expired or is invalid"""
         if self.token_info:
-            logger.info(f"Invalidating token: {self.token_info.access_token[:10]}...")
+            logger.info(f"Invalidating token: {redact_secret(self.token_info.access_token)}")
             self.token_info = None
 
             # Signal that authentication is needed
@@ -506,7 +495,7 @@ async def get_current_access_token() -> Optional[str]:
                 auth_manager.invalidate_token()
                 return None
 
-            logger.debug(f"Access token found in auth_manager (starts with: {token[:10]}...)")
+            logger.debug(f"Access token found in auth_manager ({redact_secret(token)})")
             return token
         else:
             logger.warning("No valid access token available in auth_manager")

meta_ads_mcp/core/authentication.py

@@ -36,7 +36,7 @@
 from . import auth
 from .auth import start_callback_server, shutdown_callback_server, auth_manager
 from .server import mcp_server
-from .utils import logger, META_APP_SECRET
+from .utils import logger, redact_secret, META_APP_SECRET
 
 # Only register the login link tool if not explicitly disabled
 ENABLE_LOGIN_LINK = not bool(os.environ.get("META_ADS_DISABLE_LOGIN_LINK", ""))
@@ -92,7 +92,7 @@ async def get_login_link(access_token: Optional[str] = None) -> str:
             return json.dumps({
                 "message": "✅ Already Authenticated", 
                 "status": "You're successfully authenticated with Meta Ads!",
-                "token_info": f"Token preview: {cached_token[:10]}...",
+                "token_info": f"Token: {redact_secret(cached_token)}",
                 "created_at": auth_manager.token_info.created_at if hasattr(auth_manager, "token_info") else None,
                 "expires_in": auth_manager.token_info.expires_in if hasattr(auth_manager, "token_info") else None,
                 "authentication_method": "meta_oauth",

meta_ads_mcp/core/callback_server.py

@@ -12,7 +12,7 @@
 from urllib.parse import urlparse, parse_qs
 from typing import Dict, Any, Optional
 
-from .utils import logger
+from .utils import logger, redact_secret
 
 # Global token container for communication between threads
 token_container = {"token": None, "expires_in": None, "user_id": None}
@@ -153,7 +153,7 @@ def _handle_oauth_callback(self):
             """
         elif code:
             # Success case - we have the authorization code
-            logger.info(f"Received authorization code: {code[:10]}...")
+            logger.info(f"Received authorization code: {redact_secret(code)}")
 
             # Store the authorization code temporarily
             # The auth module will exchange this for an access token

meta_ads_mcp/core/http_auth_integration.py

@@ -8,7 +8,7 @@
 import asyncio
 import contextvars
 from typing import Optional
-from .utils import logger
+from .utils import logger, redact_secret
 import json
 
 # Use context variables instead of thread-local storage for better async support
@@ -296,12 +296,12 @@ async def dispatch(self, request: Request, call_next):
             )
 
         if auth_token:
-            logger.debug(f"HTTP Auth Middleware: Extracted auth token: {auth_token[:10]}...")
+            logger.debug(f"HTTP Auth Middleware: Extracted auth token: {redact_secret(auth_token)}")
             logger.debug("Injecting auth token into request context")
             FastMCPAuthIntegration.set_auth_token(auth_token)
 
         if pipeboard_token:
-            logger.debug(f"HTTP Auth Middleware: Extracted Pipeboard token: {pipeboard_token[:10]}...")
+            logger.debug(f"HTTP Auth Middleware: Extracted Pipeboard token: {redact_secret(pipeboard_token)}")
             logger.debug("Injecting Pipeboard token into request context")
             FastMCPAuthIntegration.set_pipeboard_token(pipeboard_token)
 

meta_ads_mcp/core/utils.py

@@ -36,6 +36,61 @@
         print("WARNING: META_APP_SECRET environment variable is not set.", file=sys.stderr)
         print("NOTE: This is only needed to exchange a short-lived token for a long-lived one.", file=sys.stderr)
 
+# The log file sits next to the token cache and records request activity, so it
+# gets the same owner-only treatment. See GHSA-prmg-4fr3-mm6x for the cache.
+LOG_DIR_MODE = 0o700
+LOG_FILE_MODE = 0o600
+
+# Log level default. DEBUG used to be hard-coded, which (together with the root
+# handler below) wrote a great deal of request detail to a file that is never
+# rotated. Operators who need it set META_ADS_LOG_LEVEL=DEBUG.
+DEFAULT_LOG_LEVEL = "INFO"
+
+# httpx logs "HTTP Request: GET <url>" at INFO, and for the Graph API that URL
+# carries `access_token=<the operator's token>` in the query string. The old
+# logging.basicConfig() call installed a handler on the ROOT logger, so those
+# records landed in meta_ads_debug.log in full. The handler is now attached to
+# this package's logger only, and httpx is held at WARNING so the line is not
+# emitted into whatever logging the host application configures either.
+# See GHSA-r3r9-3mrh-x966.
+_URL_LOGGING_LIBRARIES = ("httpx", "httpcore")
+
+
+def restrict_permissions(path: pathlib.Path, mode: int) -> None:
+    """Narrow `path` to `mode` if it is wider, best effort.
+
+    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:
+        # Logging is not configured yet when this runs at import time, and a
+        # permissions failure must never stop the server from starting.
+        pass
+
+
+def redact_secret(value: Optional[str]) -> str:
+    """Render a credential for a log line or a tool response.
+
+    Returns the length only. Prefixes (`token[:10]`) were used before, which is
+    still credential material in a file that is never rotated, and long enough
+    to correlate one caller's requests across a shared deployment.
+    """
+    if not value:
+        return "<none>"
+    return f"<redacted:{len(value)} chars>"
+
+
+def _resolve_log_level() -> int:
+    requested = os.environ.get("META_ADS_LOG_LEVEL", DEFAULT_LOG_LEVEL).upper()
+    level = logging.getLevelName(requested)
+    return level if isinstance(level, int) else logging.INFO
+
+
 # Configure logging to file
 def setup_logging():
     """Set up logging to file for troubleshooting."""
@@ -49,21 +104,36 @@ def setup_logging():
 
     # Create directory if it doesn't exist
     log_dir = base_path / "meta-ads-mcp"
-    log_dir.mkdir(parents=True, exist_ok=True)
+    log_dir.mkdir(parents=True, exist_ok=True, mode=LOG_DIR_MODE)
+    restrict_permissions(log_dir, LOG_DIR_MODE)
 
     log_file = log_dir / "meta_ads_debug.log"
-    
-    # Configure file logger
-    logging.basicConfig(
-        level=logging.DEBUG,
-        format='%(asctime)s - %(name)s - %(levelname)s - %(message)s',
-        filename=str(log_file),
-        filemode='a'  # Append mode
-    )
+    # Create the file ourselves so it exists with 0600 before the handler opens
+    # it; narrow it afterwards in case an earlier version left it at 0644.
+    if not log_file.exists():
+        try:
+            os.close(os.open(log_file, os.O_CREAT | os.O_WRONLY | os.O_APPEND, LOG_FILE_MODE))
+        except OSError:
+            pass
+    restrict_permissions(log_file, LOG_FILE_MODE)
 
     # Create a logger
     logger = logging.getLogger("meta-ads-mcp")
-    logger.setLevel(logging.DEBUG)
+    logger.setLevel(_resolve_log_level())
+    
+    # Attach the file handler to this logger rather than the root logger, so
+    # third-party records (notably httpx's full request URLs) are not captured.
+    if not any(getattr(handler, "_meta_ads_mcp_handler", False) for handler in logger.handlers):
+        file_handler = logging.FileHandler(str(log_file))
+        file_handler._meta_ads_mcp_handler = True
+        file_handler.setFormatter(
+            logging.Formatter('%(asctime)s - %(name)s - %(levelname)s - %(message)s')
+        )
+        logger.addHandler(file_handler)
+    logger.propagate = False
+    
+    for library in _URL_LOGGING_LIBRARIES:
+        logging.getLogger(library).setLevel(logging.WARNING)
 
     # Log startup information
     logger.info(f"Logging initialized. Log file: {log_file}")

pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "hatchling.build"
 
 [project]
 name = "meta-ads-mcp"
-version = "1.0.127"
+version = "1.0.128"
 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"

server.json

@@ -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.127",
+  "version": "1.0.128",
   "remotes": [
     {
       "type": "streamable-http",