security: remediate top-10 audit findings (Phase A landed; Phase B in progress)

.github/workflows/ci.yml

@@ -21,7 +21,7 @@ jobs:
           # runner. pynput stays in the list, but the conftest installs a
           # stub when its real import fails (headless Linux has no X11).
           pip install pytest fastmcp httpx requests pynput pydantic ruff \
-                      fastapi numpy
+                      fastapi numpy pyotp 'qrcode[pil]'
       - name: Lint (ruff) — F-4 gate, config in ruff.toml
         run: ruff check .
       - name: Skill import smoke (standalone script, not a pytest module)

.gitignore

@@ -36,3 +36,9 @@ build/
 *.egg-info/
 *.egg
 *.whl
+
+# Test / coverage artifacts
+.coverage
+.coverage.*
+htmlcov/
+.pytest_cache/

codec_bridges.py

@@ -23,9 +23,19 @@
 
 MEMORY_DB = os.path.expanduser("~/.codec/memory.db")
 
-# Skills that need a terminal / GUI and must not run from a headless bridge.
-_SKIP_SKILLS = {"open_terminal", "run_command", "vibe_code", "deep_chat",
-                "memory_search", "ask_mike_to_build"}
+# C2 (Fix #2): inbound bridge messages may ONLY dispatch skills on this explicit
+# safe list — read / info / pure-compute skills with no system side effects, no
+# private-data disclosure, and no device/GUI control. Default-deny: anything not
+# listed is NOT dispatched from a bridge (try_skill returns (None, None) and the
+# caller degrades to an LLM answer — never a hard fail). This is the security
+# boundary that keeps high-power skills (terminal, python_exec, file_write,
+# pilot, process_manager, pm2_control, ax_control) unreachable from a (remote)
+# bridge, replacing the old allow-by-default `_SKIP_SKILLS` denylist which did
+# NOT exclude them. Skills are added here deliberately, one at a time.
+BRIDGE_SAFE_SKILLS = frozenset({
+    "weather", "time", "calculator", "translate", "bitcoin_price",
+    "web_search", "json_formatter", "password_generator", "qr_generator",
+})
 
 # Per-channel assistant persona (the only thing that differed between the two
 # bridges' call_llm). `{now}` is the formatted current date/time.
@@ -69,14 +79,23 @@ def load_dispatch() -> bool:
 
 
 def try_skill(text):
-    """Match a CODEC skill for `text`. Returns (skill_name, result) or
-    (None, None) — skipping terminal/GUI skills that a bridge can't run."""
+    """Match a CODEC skill for `text` and run it IF it is on BRIDGE_SAFE_SKILLS.
+
+    Returns (skill_name, result) for a dispatched safe skill, or (None, None)
+    when nothing safe matched — in which case the caller falls back to an LLM
+    answer (graceful degradation, never a hard fail). C2 fail-closed: any skill
+    not on the allowlist (every high-power skill included) is never dispatched
+    from a bridge.
+    """
     if not load_dispatch():
         return (None, None)
     try:
         skill = _check_skill(text)
         if skill:
-            if skill["name"] in _SKIP_SKILLS:
+            if skill["name"] not in BRIDGE_SAFE_SKILLS:
+                log.info(
+                    "Skill '%s' not on BRIDGE_SAFE_SKILLS — not dispatched from "
+                    "bridge (falling back to LLM)", skill["name"])
                 return (None, None)
             result = _run_skill(skill, text)
             if result:

codec_dashboard.py

@@ -2215,8 +2215,11 @@ async def web_search_endpoint(request: Request):
     "terminal",
     # File operations (read, write, append, list — path-restricted)
     "file_ops",
-    # Python execution (sandboxed, blocked dangerous imports)
-    "python_exec",
+    # NOTE: python_exec is intentionally NOT on this allowlist (audit C3).
+    # It stays a local skill but is no longer auto-firable from a chat message
+    # (pre-LLM hijack / post-LLM [SKILL:...] tag both gate on this set), so an
+    # injection-style chat message can't drive arbitrary code execution.
+    # SKILL_MCP_EXPOSE=False already keeps it off MCP.
     # Google services
     "google_calendar", "google_gmail", "google_docs",
     "google_drive", "google_sheets", "google_keep",
@@ -2265,7 +2268,6 @@ async def web_search_endpoint(request: Request):
 [SKILL:weather:weather in Paris]
 [SKILL:terminal:ls -la ~/Documents]
 [SKILL:file_ops:read file ~/notes.txt]
-[SKILL:python_exec:run python print(2**100)]
 [SKILL:pm2_control:pm2 list]
 [SKILL:google_calendar:what's on my calendar today]
 The skill's real output replaces the tag automatically — emit the tag and stop, never fabricate the result.

codec_imessage.py

@@ -60,7 +60,7 @@ def get_imessage_config(cfg):
     im = cfg.get("imessage", {})
     return {
         "enabled": im.get("enabled", True),
-        "allowed_senders": im.get("allowed_senders", []),   # empty = allow all
+        "allowed_senders": im.get("allowed_senders", []),   # empty = DENY all (fail-closed, C2)
         "blocked_senders": im.get("blocked_senders", []),
         "poll_interval": im.get("poll_interval", 3),        # seconds
         "max_response_length": im.get("max_response_length", 4000),
@@ -241,8 +241,19 @@ def _convert_apple_date(apple_date):
 
 
 # ── Sender filtering ────────────────────────────────────────────────────────
+# C2 (Fix #2): one-time "no allowlist configured" warning flag. Module-level so
+# the fail-closed deny logs ONCE per process instead of once per inbound message.
+_warned_no_allowlist = False
+
+
 def is_sender_allowed(sender, im_cfg):
-    """Check if sender is allowed based on allowlist/blocklist."""
+    """Check if sender is allowed based on allowlist/blocklist.
+
+    FAIL-CLOSED (C2): an EMPTY (or missing) ``allowed_senders`` denies ALL
+    inbound. A bridge with no configured allowlist must not respond to arbitrary
+    senders just because someone learned the handle. The operator enables replies
+    by adding their own handle to ``config.imessage.allowed_senders``.
+    """
     if not sender:
         return False
 
@@ -252,7 +263,16 @@ def is_sender_allowed(sender, im_cfg):
         return False
 
     allowed = im_cfg.get("allowed_senders", [])
-    if allowed and sender not in allowed:
+    if not allowed:
+        global _warned_no_allowlist
+        if not _warned_no_allowlist:
+            log.warning(
+                "bridge_no_allowlist: imessage allowed_senders is empty — "
+                "denying ALL inbound (fail-closed, C2). Add your handle to "
+                "config.imessage.allowed_senders to enable replies.")
+            _warned_no_allowlist = True
+        return False
+    if sender not in allowed:
         log.info(f"Sender not in allowlist: {sender}")
         return False
 

codec_oauth_provider.py

@@ -22,7 +22,6 @@
 
 import json
 import os
-import stat
 import time
 import secrets
 import threading
@@ -34,6 +33,8 @@
 
 from fastmcp.server.auth.providers.in_memory import InMemoryOAuthProvider
 
+from codec_jsonstore import atomic_write_json
+
 try:
     from codec_audit import log_event as _oauth_log_event
 except ImportError:  # pragma: no cover — audit unavailable shouldn't break OAuth
@@ -142,7 +143,8 @@ def _save(self):
         # legacy plaintext path so OAuth keeps working — operational
         # continuity > strict secret isolation.
         with self._lock:
-            blob = json.dumps(self._serialize())
+            state = self._serialize()
+            blob = json.dumps(state)
             kc_ok = False
             try:
                 from codec_keychain import set_oauth_state
@@ -159,12 +161,15 @@ def _save(self):
                     pass
                 return
 
-            # Fallback: legacy plaintext file (0600). Logged as a warning
-            # at the keychain layer; OAuth continues to function.
-            tmp = self._state_path.with_suffix(".tmp")
-            tmp.write_text(blob)
-            os.chmod(tmp, stat.S_IRUSR | stat.S_IWUSR)  # 0600
-            os.replace(tmp, self._state_path)
+            # Fallback: legacy plaintext file. Crash-durable write via the
+            # canonical jsonstore helper — unique tmp + flush + os.fsync +
+            # atomic os.replace + chmod 0600. C6 (Fix #1b): the previous
+            # `tmp.write_text(blob); os.replace(...)` skipped fsync, so a
+            # crash between write() and the page-cache flush could land a
+            # truncated/empty oauth_state.json and lose every token
+            # (claude.ai forced re-auth on next restart). atomic_write_json
+            # closes that durability window.
+            atomic_write_json(self._state_path, state)
 
     # ---------- overrides: persist after every mutation ----------
 

codec_sandbox.py

@@ -27,6 +27,7 @@
 ;; Allow reading most files (skills need imports)
 (allow file-read*)
 
+{read_deny_rules}
 ;; Allow writing ONLY to skill output dir and temp
 (allow file-write*
     (subpath "{skill_output}")
@@ -65,6 +66,30 @@
 (deny network-inbound)
 """
 
+# C3 (Fix #3): targeted read-denies layered AFTER the broad (allow file-read*).
+# SBPL is last-match-wins, so these win over the broad allow for these specific
+# paths while leaving stdlib / site-packages / interpreter reads working (the
+# "don't break legitimate imports" constraint). Closes the python_exec /
+# sandboxed-skill secret-exfil vector: ~/.ssh, ~/.aws, ~/.gnupg, ~/.config/gh,
+# the macOS Keychain dirs, and the ~/.codec OAuth-token + fallback-secret files.
+# NOTE: config.json is deliberately NOT denied — a sandboxed skill may
+# `import codec_config`, which reads it at import time (its secrets are
+# Keychain-backed since PR-2B, so config.json is no longer a secret store).
+_READ_DENY_TEMPLATE = """\
+;; C3: deny reads of credential / secret paths (last-match-wins over the
+;; broad allow above). Leaves stdlib imports intact.
+(deny file-read*
+    (subpath "{home}/.ssh")
+    (subpath "{home}/.aws")
+    (subpath "{home}/.gnupg")
+    (subpath "{home}/.config/gh")
+    (subpath "{home}/Library/Keychains")
+    (subpath "/Library/Keychains")
+    (literal "{codec_dir}/oauth_state.json")
+    (literal "{codec_dir}/secret.key")
+    (literal "{codec_dir}/secrets.enc.json"))
+"""
+
 
 def _ensure_dirs():
     os.makedirs(SKILL_OUTPUT_DIR, exist_ok=True)
@@ -75,9 +100,14 @@ def _write_sandbox_profile(allow_network: bool = False) -> str:
     """Write a sandbox.sb profile and return its path."""
     _ensure_dirs()
     network_rules = _NETWORK_ALLOW if allow_network else _NETWORK_DENY
+    read_deny_rules = _READ_DENY_TEMPLATE.format(
+        home=os.path.expanduser("~"),
+        codec_dir=_CODEC_DIR,
+    )
     profile = _SANDBOX_PROFILE_TEMPLATE.format(
         skill_output=SKILL_OUTPUT_DIR,
         network_rules=network_rules,
+        read_deny_rules=read_deny_rules,
     )
     with open(SANDBOX_PROFILE_PATH, "w") as f:
         f.write(profile)

codec_telegram.py

@@ -67,7 +67,7 @@ def get_telegram_config(cfg):
         bot_token = tg.get("bot_token", "")
     return {
         "bot_token": bot_token,
-        "allowed_chat_ids": tg.get("allowed_chat_ids", []),  # empty = allow all
+        "allowed_chat_ids": tg.get("allowed_chat_ids", []),  # empty = DENY all (fail-closed, C2)
         "require_trigger": tg.get("require_trigger", False),  # DMs don't need trigger by default
         "max_response_length": tg.get("max_response_length", 4000),
     }
@@ -492,6 +492,33 @@ def save_to_memory(chat_id, user_text, assistant_text):
     return codec_bridges.save_to_memory("telegram", chat_id, user_text, assistant_text)
 
 
+# ── Chat filtering ──────────────────────────────────────────────────────────
+# C2 (Fix #2): one-time "no allowlist configured" warning flag (mirrors
+# codec_imessage). Module-level so the fail-closed deny logs ONCE per process.
+_warned_no_allowlist = False
+
+
+def is_chat_allowed(chat_id, tg_cfg):
+    """Whether `chat_id` may drive CODEC over the Telegram bridge.
+
+    FAIL-CLOSED (C2): an EMPTY (or missing) ``allowed_chat_ids`` denies ALL
+    inbound — anyone who learns the bot token must NOT be able to drive CODEC.
+    The operator enables replies by adding their own chat id to
+    ``config.telegram.allowed_chat_ids``.
+    """
+    allowed = tg_cfg.get("allowed_chat_ids", [])
+    if not allowed:
+        global _warned_no_allowlist
+        if not _warned_no_allowlist:
+            log.warning(
+                "bridge_no_allowlist: telegram allowed_chat_ids is empty — "
+                "denying ALL inbound (fail-closed, C2). Add your chat id to "
+                "config.telegram.allowed_chat_ids to enable replies.")
+            _warned_no_allowlist = True
+        return False
+    return chat_id in allowed
+
+
 # ── Process message ─────────────────────────────────────────────────────────
 def process_message(bot, update, tg_cfg, llm_cfg):
     msg = update.get("message", {})
@@ -504,10 +531,9 @@ def process_message(bot, update, tg_cfg, llm_cfg):
     if not chat_id:
         return
 
-    # Chat ID filter
-    allowed = tg_cfg.get("allowed_chat_ids", [])
-    if allowed and chat_id not in allowed:
-        log.info(f"Chat {chat_id} not in allowlist — ignored")
+    # Chat ID filter — FAIL-CLOSED (C2): empty allowed_chat_ids denies all.
+    if not is_chat_allowed(chat_id, tg_cfg):
+        log.info(f"Chat {chat_id} not allowed — ignored")
         return
 
     # ── Extract text ─────────────────────────────────────────────────────