From 763737110051880b6155cf976dba3f7e5ff87bc5 Mon Sep 17 00:00:00 2001
From: Kurtisone <104103601+Kurtisone@users.noreply.github.com>
Date: Wed, 12 Aug 2026 15:48:34 +0000
Subject: [PATCH 1/5] fix(ui): close stored XSS in the markdown renderer (audit
 C-3)

escapeHtml() escaped only &, < and >, but its output lands in
attribute position -- inlineMarkdown built <a href="$2"> by direct
regex interpolation. `[x](" autofocus onfocus="location=name)`
therefore rendered as a valid <a> with an injected event handler and
executed with no interaction. A `javascript:` href needed no quote at
all.

This matters beyond the model's own output: the text passed to
formatContent() includes tool results -- web_fetch/research page
content, files:read, sysadmin logs -- so a hostile page reached it.
And the API token lives in localStorage, so the payoff was token
theft, then authenticated /chat calls, then whatever shell/files are
enabled.

Three layers, in order of how much each actually closes:
- escapeHtml now escapes " and ' too (kills the attribute escape)
- link rendering validates the scheme against an allowlist, and
  downgrades anything else to plain text (kills javascript:)
- GET / serves a CSP; 'unsafe-inline' is still required by the
  single-file UI, so this doesn't stop injected script from running
  -- but connect-src 'self' stops it shipping the token off-host

The two JS fixes are covered by source assertions rather than
execution: a regression tripwire, not a proof, and honest about it in
the test module docstring. The headers are tested for real.
---
 src/forge/api.py            | 33 +++++++++++++++--
 src/forge/static/index.html | 32 +++++++++++++++--
 tests/test_ui_security.py   | 70 +++++++++++++++++++++++++++++++++++++
 3 files changed, 131 insertions(+), 4 deletions(-)
 create mode 100644 tests/test_ui_security.py

diff --git a/src/forge/api.py b/src/forge/api.py
index fe0179e..ee56314 100644
--- a/src/forge/api.py
+++ b/src/forge/api.py
@@ -434,11 +434,40 @@ async def compact():
 # ─── UI ────────────────────────────────────────────────────────────
 
 
+# Third layer under the two fixes in static/index.html (quote escaping
+# + link-scheme validation): defense in depth, not a replacement for
+# them. 'unsafe-inline' is unavoidable for now -- the UI is a single
+# self-contained file with inline <script>/<style> and onclick=
+# attributes, deliberately (no CDN, must work offline). So this does
+# NOT stop an inline XSS from running. What it does stop is the part
+# that actually hurts: connect-src 'self' means injected script can't
+# POST the localStorage API token to an attacker's host, and
+# default-src 'self' blocks pulling a payload from anywhere external.
+# Splitting the JS into its own static file would let 'unsafe-inline'
+# go away entirely -- worth doing, but a bigger change than this fix.
+_CSP = (
+    "default-src 'self'; "
+    "script-src 'self' 'unsafe-inline'; "
+    "style-src 'self' 'unsafe-inline'; "
+    "img-src 'self' data:; "
+    "connect-src 'self'; "
+    "base-uri 'none'; "
+    "form-action 'none'; "
+    "frame-ancestors 'none'"
+)
+_UI_HEADERS = {
+    "Content-Security-Policy": _CSP,
+    "X-Content-Type-Options": "nosniff",
+    "Referrer-Policy": "no-referrer",
+}
+
+
 @app.get("/", response_class=HTMLResponse)
 async def ui():
     static = Path(__file__).parent / "static" / "index.html"
     if static.exists():
-        return HTMLResponse(static.read_text(encoding="utf-8"))
+        return HTMLResponse(static.read_text(encoding="utf-8"), headers=_UI_HEADERS)
     return HTMLResponse(
-        "<h1>Forge UI not found</h1><p>Run from the installed package.</p>"
+        "<h1>Forge UI not found</h1><p>Run from the installed package.</p>",
+        headers=_UI_HEADERS,
     )
diff --git a/src/forge/static/index.html b/src/forge/static/index.html
index eaf94f2..e636162 100644
--- a/src/forge/static/index.html
+++ b/src/forge/static/index.html
@@ -532,11 +532,23 @@
   return working.replace(/\x00BLOCK(\d+)\x00/g, (_, i) => blocks[Number(i)])
 }
 
+// Only these schemes may ever become an href. escapeHtml() now closes
+// the attribute-escape route, but a *syntactically valid* href is
+// still dangerous on its own: `[clique ici](javascript:alert)` needs
+// no quote at all, it just needs to be rendered as a link. Anything
+// not matching is downgraded to plain text rather than dropped, so a
+// legitimate-but-unusual URL is still readable instead of vanishing.
+const _SAFE_LINK_SCHEME = /^(https?:\/\/|mailto:|\/|#)/i
+
 function inlineMarkdown(s) {
   return s
     .replace(/\*\*([^*]+)\*\*/g, '<strong>$1</strong>')
     .replace(/(?<!\*)\*([^*]+)\*(?!\*)/g, '<em>$1</em>')
-    .replace(/\[([^\]]+)\]\(([^)]+)\)/g, '<a href="$2" target="_blank" rel="noopener">$1</a>')
+    .replace(/\[([^\]]+)\]\(([^)]+)\)/g, (_, label, url) => {
+      const href = url.trim()
+      if (!_SAFE_LINK_SCHEME.test(href)) return `${label} (${href})`
+      return `<a href="${href}" target="_blank" rel="noopener noreferrer">${label}</a>`
+    })
 }
 
 // ```diff blocks: colored like `git diff` instead of plain text, so a
@@ -552,8 +564,24 @@
   return `<pre class="diff">${lines.join('\n')}</pre>`
 }
 
+// Quotes matter as much as angle brackets here: this output lands in
+// ATTRIBUTE position in several places (inlineMarkdown's <a href="...">
+// above, the trace/drawer template literals below), so escaping only
+// &/</> leaves a straight attribute escape open. Concretely, before
+// this: `[x](" autofocus onfocus="location=name)` rendered as
+// <a href="" autofocus onfocus="location=name" ...> and executed with
+// no interaction at all -- and the text reaching formatContent()
+// includes tool output (web_fetch/research page content, files:read,
+// sysadmin logs), so a hostile page was enough to reach it. The API
+// token lives in localStorage, so that XSS was a token theft, not a
+// cosmetic bug.
 function escapeHtml(s) {
-  return s.replace(/&/g,'&amp;').replace(/</g,'&lt;').replace(/>/g,'&gt;')
+  return s
+    .replace(/&/g, '&amp;')
+    .replace(/</g, '&lt;')
+    .replace(/>/g, '&gt;')
+    .replace(/"/g, '&quot;')
+    .replace(/'/g, '&#39;')
 }
 
 function showThinking() {
diff --git a/tests/test_ui_security.py b/tests/test_ui_security.py
new file mode 100644
index 0000000..53720c1
--- /dev/null
+++ b/tests/test_ui_security.py
@@ -0,0 +1,70 @@
+"""
+Tests for the UI's XSS hardening (security audit, C-3).
+
+Two of the three fixes live in JavaScript inside static/index.html,
+which this suite has no runtime for -- so those two are covered by
+asserting on the source itself. That is a weaker test than executing
+the renderer, and deliberately so: it's a regression tripwire, not a
+proof. It catches the realistic failure mode (someone simplifies
+escapeHtml back to three replaces, or rewrites the link rule as a
+plain string interpolation) without pulling a JS engine into the
+test dependencies for one file.
+
+The third fix (response headers) is a real behavioural test.
+"""
+
+from pathlib import Path
+
+from fastapi.testclient import TestClient
+
+import forge.api as api_mod
+
+_INDEX = Path(api_mod.__file__).parent / "static" / "index.html"
+
+
+def _index_source() -> str:
+    return _INDEX.read_text(encoding="utf-8")
+
+
+# ── escapeHtml covers attribute context ──────────────────────────────
+
+
+def test_escape_html_escapes_quotes():
+    src = _index_source()
+    assert "&quot;" in src, "escapeHtml must escape double quotes"
+    assert "&#39;" in src, "escapeHtml must escape single quotes"
+
+
+# ── link rendering validates the scheme ──────────────────────────────
+
+
+def test_link_rule_is_not_a_raw_interpolation():
+    """
+    The vulnerable form was a direct regex replacement string:
+        '<a href="$2" target="_blank" rel="noopener">$1</a>'
+    which put unvalidated URL text straight into an attribute.
+    """
+    assert '<a href="$2"' not in _index_source()
+
+
+def test_link_scheme_allowlist_exists():
+    assert "_SAFE_LINK_SCHEME" in _index_source()
+
+
+# ── response headers ─────────────────────────────────────────────────
+
+
+def test_ui_sends_content_security_policy():
+    r = TestClient(api_mod.app).get("/")
+    assert r.status_code == 200
+    csp = r.headers.get("content-security-policy")
+    assert csp is not None
+    # The clause that matters most: an injected script cannot ship the
+    # localStorage API token off-host.
+    assert "connect-src 'self'" in csp
+    assert "default-src 'self'" in csp
+
+
+def test_ui_sends_nosniff():
+    r = TestClient(api_mod.app).get("/")
+    assert r.headers.get("x-content-type-options") == "nosniff"

From 3743fc7e08f873e6583921060f830bd093d875bb Mon Sep 17 00:00:00 2001
From: Kurtisone <104103601+Kurtisone@users.noreply.github.com>
Date: Wed, 12 Aug 2026 16:40:53 +0000
Subject: [PATCH 2/5] feat(api): refuse to start unauthenticated unless opted
 into (audit C-1)

API_TOKEN defaulted to empty, which left /chat, /run, /review,
/traces, /tools and the memory endpoints open to anyone reaching the
port -- and /chat dispatches whatever is in ENABLED_TOOLS. Combined
with the Containerfile's `--host 0.0.0.0`, "open by default" means
arbitrary tool execution by default.

config.py already carried a comment warning about this. A documented
unsafe default is still an unsafe default: it's the kind you notice
the day you add a published port to a compose file, not before.

So the default is inverted rather than re-documented. Starting with no
token now raises InsecureConfiguration from the app's lifespan, unless
API_ALLOW_UNAUTHENTICATED=true records in .env.local that the open
posture is intentional. The error names both ways out -- a refusal
that doesn't say how to proceed just gets worked around badly.

Nothing changes for the auth path itself: require_token() is
untouched, and the existing test suite (bare TestClient(app), which
doesn't run lifespan) still passes as-is.
---
 .env.example                    | 13 ++++++--
 src/forge/api.py                | 54 ++++++++++++++++++++++++++++++---
 src/forge/config.py             |  9 ++++++
 tests/test_api_startup_guard.py | 54 +++++++++++++++++++++++++++++++++
 4 files changed, 123 insertions(+), 7 deletions(-)
 create mode 100644 tests/test_api_startup_guard.py

diff --git a/.env.example b/.env.example
index b3c4124..c68f785 100644
--- a/.env.example
+++ b/.env.example
@@ -132,10 +132,17 @@
 # SHOW_DEBUG=false
 
 # --- API auth ---------------------------------------------------------------
-# Empty = the HTTP API (/chat, /review, /run, /tools, /traces, /remember,
-# /search) stays open.
-# Set this before exposing forge-core beyond localhost or a trusted LAN.
+# Bearer token required on /chat, /review, /run, /tools, /traces,
+# /remember, /search, /history, /drawer, /compact.
+# Forge REFUSES TO START if this is empty, unless you also set
+# API_ALLOW_UNAUTHENTICATED=true below. That's deliberate: /chat
+# dispatches whatever is in ENABLED_TOOLS, so an open instance is
+# arbitrary tool execution for anyone who can reach the port.
 # API_TOKEN=
+#
+# Set to true only for a genuinely local-only instance you want open on
+# purpose. Never set it on anything reachable over WireGuard/LAN.
+# API_ALLOW_UNAUTHENTICATED=false
 
 # --- API rate limiting -------------------------------------------------------
 # In-memory sliding window, per client IP, single-process only.
diff --git a/src/forge/api.py b/src/forge/api.py
index ee56314..b8f4647 100644
--- a/src/forge/api.py
+++ b/src/forge/api.py
@@ -15,8 +15,9 @@
 
 Auth: set API_TOKEN in the environment to require
 `Authorization: Bearer <token>` on every endpoint except / and
-/health. Unset (default) means the API stays open, unchanged from
-before this was added.
+/health. Leaving it unset no longer silently opens the API -- the app
+refuses to start unless API_ALLOW_UNAUTHENTICATED=true says the open
+posture is intentional. See check_auth_configuration() below.
 
 Rate limiting: in-memory sliding window, per client IP, on every
 endpoint except / and /health. RATE_LIMIT_REQUESTS per
@@ -33,6 +34,7 @@
 import asyncio
 import hmac
 from concurrent.futures import ThreadPoolExecutor
+from contextlib import asynccontextmanager
 from pathlib import Path
 from typing import Literal
 
@@ -41,10 +43,54 @@
 from pydantic import BaseModel
 
 from forge import rag, ratelimit, trace
-from forge.config import API_TOKEN, FORGE_PROVIDER, LLAMA_CPP_URL, LLM_MODEL
+from forge.config import (
+    API_ALLOW_UNAUTHENTICATED,
+    API_TOKEN,
+    FORGE_PROVIDER,
+    LLAMA_CPP_URL,
+    LLM_MODEL,
+)
 from forge.orchestrator import Orchestrator
 
-app = FastAPI(title="Forge", version="3.3.0", docs_url="/docs")
+
+class InsecureConfiguration(RuntimeError):
+    """Raised at startup when the API would come up unauthenticated
+    without anyone having asked for that in writing."""
+
+
+def check_auth_configuration() -> None:
+    """
+    Refuse to start with no API_TOKEN unless API_ALLOW_UNAUTHENTICATED
+    is explicitly set.
+
+    /chat dispatches whatever is in ENABLED_TOOLS -- shell, files, test,
+    sysadmin -- and the container's CMD binds 0.0.0.0. So "no token" is
+    not a mild default: it's arbitrary tool dispatch for anyone who can
+    reach the port. Failing loudly at startup is the only version of
+    this warning that can't be scrolled past.
+
+    Reads the module globals rather than the config constants directly
+    so tests (and anything embedding the app) can patch them at the
+    same boundary the auth dependency already uses.
+    """
+    if API_TOKEN or API_ALLOW_UNAUTHENTICATED:
+        return
+    raise InsecureConfiguration(
+        "refusing to start: API_TOKEN is unset, so /chat, /run and every "
+        "other endpoint would accept unauthenticated requests -- including "
+        "tool dispatch. Set API_TOKEN in .env.local, or set "
+        "API_ALLOW_UNAUTHENTICATED=true if this instance really is "
+        "local-only and you want it open on purpose."
+    )
+
+
+@asynccontextmanager
+async def lifespan(_app: FastAPI):
+    check_auth_configuration()
+    yield
+
+
+app = FastAPI(title="Forge", version="3.3.0", docs_url="/docs", lifespan=lifespan)
 _executor = ThreadPoolExecutor(max_workers=2)
 _orchestrator = Orchestrator()
 
diff --git a/src/forge/config.py b/src/forge/config.py
index 2cc6239..c8c040b 100644
--- a/src/forge/config.py
+++ b/src/forge/config.py
@@ -161,6 +161,15 @@ def _bool(name: str, default: str = "false") -> bool:
 # -- /chat, /review, /run, /traces and /tools currently have zero
 # protection otherwise.
 API_TOKEN = os.getenv("API_TOKEN", "")
+# Forge refuses to start with no API_TOKEN unless this is set to true.
+# The old behaviour was "open by default, documented as risky" -- but a
+# documented unsafe default is still an unsafe default, and this one is
+# the kind you notice the day you add a published port to a compose
+# file, not before. Flipping it means the risky configuration has to be
+# written down in .env.local, where it's visible, instead of being what
+# happens when you write nothing at all. Local-only development is a
+# perfectly good reason to set it; forgetting isn't.
+API_ALLOW_UNAUTHENTICATED = _bool("API_ALLOW_UNAUTHENTICATED", "false")
 
 # --- API rate limiting ----------------------------------------------------
 # In-memory sliding window, per client IP, single-process only (see
diff --git a/tests/test_api_startup_guard.py b/tests/test_api_startup_guard.py
new file mode 100644
index 0000000..5f78219
--- /dev/null
+++ b/tests/test_api_startup_guard.py
@@ -0,0 +1,54 @@
+"""
+Tests for the startup auth guard (security audit, C-1).
+
+The guard runs in the app's lifespan, so it fires on a real uvicorn
+boot and on TestClient used as a context manager -- not on a bare
+TestClient(app), which is why the rest of the suite (which builds
+clients that way, with API_TOKEN monkeypatched per-test) is
+unaffected.
+"""
+
+import pytest
+from fastapi.testclient import TestClient
+
+import forge.api as api_mod
+
+
+def test_refuses_when_token_missing_and_not_opted_out(monkeypatch):
+    monkeypatch.setattr(api_mod, "API_TOKEN", "")
+    monkeypatch.setattr(api_mod, "API_ALLOW_UNAUTHENTICATED", False)
+    with pytest.raises(api_mod.InsecureConfiguration):
+        api_mod.check_auth_configuration()
+
+
+def test_allows_when_token_is_set(monkeypatch):
+    monkeypatch.setattr(api_mod, "API_TOKEN", "s3cret")
+    monkeypatch.setattr(api_mod, "API_ALLOW_UNAUTHENTICATED", False)
+    api_mod.check_auth_configuration()  # must not raise
+
+
+def test_allows_when_explicitly_opted_out(monkeypatch):
+    monkeypatch.setattr(api_mod, "API_TOKEN", "")
+    monkeypatch.setattr(api_mod, "API_ALLOW_UNAUTHENTICATED", True)
+    api_mod.check_auth_configuration()  # must not raise
+
+
+def test_error_message_names_both_ways_out(monkeypatch):
+    """
+    A refusal that doesn't say how to proceed just gets worked around
+    by whatever the first search result suggests.
+    """
+    monkeypatch.setattr(api_mod, "API_TOKEN", "")
+    monkeypatch.setattr(api_mod, "API_ALLOW_UNAUTHENTICATED", False)
+    with pytest.raises(api_mod.InsecureConfiguration) as excinfo:
+        api_mod.check_auth_configuration()
+    message = str(excinfo.value)
+    assert "API_TOKEN" in message
+    assert "API_ALLOW_UNAUTHENTICATED" in message
+
+
+def test_lifespan_actually_runs_the_check(monkeypatch):
+    monkeypatch.setattr(api_mod, "API_TOKEN", "")
+    monkeypatch.setattr(api_mod, "API_ALLOW_UNAUTHENTICATED", False)
+    with pytest.raises(api_mod.InsecureConfiguration), TestClient(api_mod.app):
+        pass  # pragma: no cover -- entering the context is what raises

From af86884b403e1f48f258c5b75055ae788e9744ae Mon Sep 17 00:00:00 2001
From: Kurtisone <104103601+Kurtisone@users.noreply.github.com>
Date: Wed, 12 Aug 2026 16:42:06 +0000
Subject: [PATCH 3/5] fix(shell): drop interpreters from the default allowlist
 (audit C-2)

The default was ls,cat,head,tail,wc,grep,find,python3,pip,pytest.
Three of those defeat the allowlist entirely, because only parts[0] is
ever checked:

    python3 -c "import os; os.system(...)"
    pip install <url>            (setup.py runs, plus network egress)
    find . -exec <anything> {} \;

An allowlist containing an interpreter is not an allowlist. The
default is now ls,cat,head,tail,wc,grep. Putting python3 back still
works and is a legitimate choice on a trusted local box -- it just has
to be written down in .env.local rather than being what you get by
default, and it now logs a warning at import naming what was switched
off.

Also corrects the module docstring, which claimed cwd=WORKSPACE_DIR
meant "relative paths cannot escape to the host filesystem". The cwd
confines nothing: `cat /etc/passwd` always worked. That sentence is
how an allowlist gets widened "safely", so it's replaced with an
explicit note that the allowlist is the only real boundary here --
and that HOME still points at the real home directory.
---
 .env.example             |  8 ++++++-
 src/forge/config.py      | 46 +++++++++++++++++++++++++++++++++++-
 src/forge/tools/shell.py | 51 +++++++++++++++++++++++++++++++++-------
 tests/test_shell_git.py  | 27 +++++++++++++++++++--
 4 files changed, 119 insertions(+), 13 deletions(-)

diff --git a/.env.example b/.env.example
index c68f785..1e04ec2 100644
--- a/.env.example
+++ b/.env.example
@@ -58,7 +58,13 @@
 # ENABLED_TOOLS=chat,code,files,shell,git,memory,review,test,web_fetch,web_search,research
 # WORKSPACE_DIR=data/workspace
 # SHELL_TIMEOUT=30
-# SHELL_ALLOWED_COMMANDS=ls,cat,head,tail,wc,grep,find,python3,pip,pytest
+# Only parts[0] is checked, never the arguments -- so adding an
+# interpreter (python3, pip, find, xargs, sh, git, sed, awk...) makes
+# this list decorative: `python3 -c "..."` runs anything. That's a
+# reasonable thing to do on a trusted local box, and a bad thing to do
+# on anything reachable over WireGuard. A warning is logged at startup
+# when one of them is present.
+# SHELL_ALLOWED_COMMANDS=ls,cat,head,tail,wc,grep
 
 # --- Test/lint tool ---------------------------------------------------------
 # Separate allowlist from the general shell tool above, on purpose --
diff --git a/src/forge/config.py b/src/forge/config.py
index c8c040b..c72c99c 100644
--- a/src/forge/config.py
+++ b/src/forge/config.py
@@ -110,12 +110,56 @@ def _bool(name: str, default: str = "false") -> bool:
 
 # --- Shell tool -------------------------------------------------------------
 SHELL_TIMEOUT = int(os.getenv("SHELL_TIMEOUT", "30"))
-_default_shell_cmds = "ls,cat,head,tail,wc,grep,find,python3,pip,pytest"
+# The default no longer includes python3, pip or find. Each of those is
+# an interpreter in its own right, so allowlisting it allowlists
+# everything:
+#   python3 -c "import os; os.system(...)"
+#   pip install <url>            (runs setup.py, plus network egress)
+#   find . -exec <anything> {} \;
+# The allowlist only checks parts[0], never the arguments -- an
+# allowlist containing an interpreter is not an allowlist.
+#
+# _SHELL_ALLOWLIST_DEFEATING below is the same idea generalised: adding
+# any of those to SHELL_ALLOWED_COMMANDS is a decision to disable this
+# protection, which is legitimate on a trusted local box and should be
+# a deliberate act, not a default. tools/shell.py logs a warning at
+# import when one is present, so the choice is visible in the logs.
+_default_shell_cmds = "ls,cat,head,tail,wc,grep"
 SHELL_ALLOWED_COMMANDS: set[str] = {
     c.strip()
     for c in os.getenv("SHELL_ALLOWED_COMMANDS", _default_shell_cmds).split(",")
     if c.strip()
 }
+# Not exhaustive and can't be: this is a "you are switching the
+# allowlist off" tripwire, not a blocklist. Anything that can execute
+# an arbitrary argument belongs here.
+_SHELL_ALLOWLIST_DEFEATING: set[str] = {
+    "awk",
+    "bash",
+    "env",
+    "find",
+    "gawk",
+    "git",
+    "less",
+    "man",
+    "more",
+    "nano",
+    "nc",
+    "perl",
+    "pip",
+    "pip3",
+    "python",
+    "python3",
+    "ruby",
+    "sed",
+    "sh",
+    "ssh",
+    "tar",
+    "vi",
+    "vim",
+    "xargs",
+    "zsh",
+}
 
 # --- Test/lint tool ----------------------------------------------------------
 # Separate from SHELL_ALLOWED_COMMANDS on purpose: the test tool has its own
diff --git a/src/forge/tools/shell.py b/src/forge/tools/shell.py
index 5afca66..8901608 100644
--- a/src/forge/tools/shell.py
+++ b/src/forge/tools/shell.py
@@ -1,33 +1,66 @@
 """
-Sandboxed shell tool.
+Allowlisted shell tool.
 
-Executes commands in a subprocess confined to WORKSPACE_DIR.
-Three layers of protection:
+Executes commands in a subprocess. Two real protections, and one
+convenience that is often mistaken for a third:
 
 1. Allowlist: only commands in SHELL_ALLOWED_COMMANDS are accepted.
+   This is the ONLY thing standing between router output and
+   arbitrary execution, and it is worth exactly as much as the
+   binaries on the list -- only parts[0] is checked, never the
+   arguments, so a single interpreter on the list (python3, find,
+   pip, xargs, ...) makes the whole allowlist decorative. See
+   _SHELL_ALLOWLIST_DEFEATING in config.py; a warning is logged at
+   import when one is configured.
 2. Timeout: execution is hard-killed after SHELL_TIMEOUT seconds.
-3. Working directory: all commands run with cwd=WORKSPACE_DIR so
-   relative paths cannot escape to the host filesystem.
+3. NOT a sandbox: commands run with cwd=WORKSPACE_DIR. This makes
+   relative paths land in the workspace, which is convenient, but it
+   confines nothing at all -- `cat /etc/passwd` and `grep -r . /`
+   work exactly as they would anywhere else. An earlier version of
+   this docstring claimed the cwd stopped commands escaping to the
+   host filesystem; it does not, and reading it that way is how an
+   allowlist gets widened "safely".
 
 The environment passed to the subprocess is minimal (PATH, HOME,
-PYTHONPATH) — no credentials, no tokens, no host env variables.
+PYTHONPATH) -- no credentials, no tokens, no host env variables.
+Note HOME still points at the real home directory, so a command able
+to read files can read ~/.ssh: another reason the allowlist matters
+more than the cwd.
 
 To activate: ENABLED_TOOLS=chat,code,shell in .env.local
-To customise: SHELL_ALLOWED_COMMANDS=ls,cat,python3
+To customise: SHELL_ALLOWED_COMMANDS=ls,cat,grep
 
 Interface: run(content: str) -> str
-  content is a plain command string: "ls -la" / "python3 script.py"
+  content is a plain command string: "ls -la" / "grep -n foo bar.py"
 """
 
 import shlex
 import subprocess
 from pathlib import Path
 
-from forge.config import SHELL_ALLOWED_COMMANDS, SHELL_TIMEOUT, WORKSPACE_DIR
+from forge.config import (
+    _SHELL_ALLOWLIST_DEFEATING,
+    SHELL_ALLOWED_COMMANDS,
+    SHELL_TIMEOUT,
+    WORKSPACE_DIR,
+)
 from forge.logger import log
 
 _MAX_OUTPUT_CHARS = 8_000
 
+# Logged once, at import, rather than on every run: this is a
+# configuration fact, not a per-call event, and burying it in the
+# output of each command is how people learn to ignore it.
+_defeating = sorted(SHELL_ALLOWED_COMMANDS & _SHELL_ALLOWLIST_DEFEATING)
+if _defeating:
+    log.warning(
+        "shell: SHELL_ALLOWED_COMMANDS contains %s -- each of these can execute "
+        "arbitrary commands via its own arguments, so the allowlist no longer "
+        "restricts anything. Intentional on a trusted local box; never on an "
+        "instance reachable from the network.",
+        ", ".join(_defeating),
+    )
+
 
 def _safe_cwd() -> Path:
     cwd = Path(WORKSPACE_DIR).resolve()
diff --git a/tests/test_shell_git.py b/tests/test_shell_git.py
index 277cff2..3ed2ff2 100644
--- a/tests/test_shell_git.py
+++ b/tests/test_shell_git.py
@@ -39,15 +39,38 @@ def test_shell_empty_command(tmp_path, monkeypatch):
     assert "[error]" in r
 
 
-def test_shell_python(tmp_path, monkeypatch):
+def test_shell_python_is_not_allowed_by_default():
+    """
+    python3 used to ship in the default allowlist, which made the whole
+    allowlist decorative -- `python3 -c "..."` runs anything. The
+    default is now interpreter-free (audit C-2).
+    """
+    assert "python3" not in cfg.SHELL_ALLOWED_COMMANDS
+    assert "pip" not in cfg.SHELL_ALLOWED_COMMANDS
+    assert "find" not in cfg.SHELL_ALLOWED_COMMANDS
+
+
+def test_shell_python_still_works_when_explicitly_allowed(tmp_path, monkeypatch):
+    """
+    Removing it from the default must not remove the capability: an
+    operator who puts python3 back gets python3. The point of C-2 is
+    that this becomes a written-down choice, not the out-of-the-box
+    state.
+    """
     monkeypatch.setattr(cfg, "WORKSPACE_DIR", str(tmp_path))
-    monkeypatch.setattr(cfg, "SHELL_ALLOWED_COMMANDS", {"python3"})
     monkeypatch.setattr(cfg, "SHELL_TIMEOUT", 10)
     monkeypatch.setattr(shell_mod, "WORKSPACE_DIR", str(tmp_path))
+    monkeypatch.setattr(shell_mod, "SHELL_ALLOWED_COMMANDS", {"python3"})
+    monkeypatch.setattr(shell_mod, "SHELL_TIMEOUT", 10)
     r = shell_mod.run('python3 -c "print(1+1)"')
     assert "2" in r
 
 
+def test_allowlist_defeating_set_covers_the_obvious_interpreters():
+    for name in ("python3", "pip", "find", "sh", "bash", "xargs", "env"):
+        assert name in cfg._SHELL_ALLOWLIST_DEFEATING
+
+
 # ── git ────────────────────────────────────────────────────────────
 
 

From 34611a18bc76c0b7d962e56388b4e5a03ce66a49 Mon Sep 17 00:00:00 2001
From: Kurtisone <104103601+Kurtisone@users.noreply.github.com>
Date: Wed, 12 Aug 2026 16:43:03 +0000
Subject: [PATCH 4/5] fix(git): remove stash, filter write/exec arguments,
 restrict env (audit E-3)

Three things, all in a tool whose docstring said "All operations are
read-only":

- `stash` was on the allowlist. Bare `git stash` is `git stash push`,
  which removes uncommitted changes from the working tree, and
  `stash drop`/`clear` destroy existing stashes. Not exfiltration, but
  on a project developed live it costs a session. Removed.

- Arguments were never inspected, only the subcommand. git has options
  that write files or run programs regardless of subcommand
  (--output, --exec, --upload-pack, --ext-diff), which turn a
  read-only subcommand into a write or an execution. Now refused by
  prefix, so both --output=x and --output x are caught.

- No env was passed to subprocess.run, unlike shell.py and test.py, so
  the git subprocess inherited the whole API process environment --
  API_TOKEN and OPENROUTER_API_KEY included. Now the same minimal env
  as its sibling tools, plus GIT_TERMINAL_PROMPT=0 so a repo asking
  for credentials fails instead of hanging until the timeout.

Not addressed here: _find_git_root() still walks up from cwd, so
inside the container this tool describes Forge's own /app repo rather
than the workspace. That's a behaviour change with real UX
consequences, so it's left for a separate decision rather than
smuggled into a security fix.
---
 src/forge/tools/git.py  | 56 +++++++++++++++++++++++++++++++++++++----
 tests/test_shell_git.py | 38 ++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+), 5 deletions(-)

diff --git a/src/forge/tools/git.py b/src/forge/tools/git.py
index abeba75..e4678fd 100644
--- a/src/forge/tools/git.py
+++ b/src/forge/tools/git.py
@@ -1,17 +1,30 @@
 """
 Git read-only tool.
 
-Runs a curated set of git commands in the current working directory.
-All operations are read-only — no commits, no push, no checkout.
+Runs a curated set of git subcommands. All of them are genuinely
+read-only — no commits, no push, no checkout, and nothing that
+touches the working tree or the stash.
 
-The allowed git subcommands are hard-coded here (not configurable)
+The allowed subcommands are hard-coded here (not configurable)
 because git commands that mutate state require user confirmation, not
 an LLM routing decision.
 
+`stash` used to be on this list, under the same "read-only" heading.
+It isn't: bare `git stash` is `git stash push`, which removes
+uncommitted changes from the working tree, and `stash drop`/`clear`
+destroy existing stashes. Nothing here was going to exfiltrate
+anything through it — but on a project developed live, losing a
+working tree costs a session. Removed.
+
+A few flags are refused regardless of subcommand: git has options
+that write files or execute programs (--output, --exec,
+--upload-pack, --ext-diff...), which would turn a read-only
+subcommand into a write or an execution.
+
 To activate: ENABLED_TOOLS=chat,code,git in .env.local
 
 Interface: run(content: str) -> str
-  content is one of: status / diff / log / show / branch / stash
+  content is one of: status / diff / log / show / branch
   optionally with extra flags: "log --oneline -5"
 """
 
@@ -27,11 +40,20 @@
     "log",
     "show",
     "branch",
-    "stash",
     "shortlog",
     "describe",
     "rev-parse",
 }
+# Refused wherever they appear in the arguments. Prefix-matched, so
+# both "--output=x" and "--output x" are caught.
+_BLOCKED_ARG_PREFIXES = (
+    "--output",
+    "--exec",
+    "--upload-pack",
+    "--receive-pack",
+    "--ext-diff",
+    "-o",
+)
 _TIMEOUT = 15
 _MAX_OUTPUT_CHARS = 6_000
 
@@ -41,6 +63,20 @@
     "diff": ["--stat"],
 }
 
+# Same minimal-env posture as tools/shell.py and tools/test.py, which
+# this module was missing: without it the subprocess inherited the
+# whole host environment, API_TOKEN and OPENROUTER_API_KEY included.
+# GIT_* config vars are excluded along with everything else, which is
+# what we want -- no ambient credential helper, no ambient identity.
+_ENV = {
+    "PATH": "/usr/local/bin:/usr/bin:/bin",
+    "HOME": str(Path.home()),
+    "TERM": "dumb",
+    # Read-only commands should never prompt; if a repo somehow asks
+    # for credentials, fail instead of hanging until the timeout.
+    "GIT_TERMINAL_PROMPT": "0",
+}
+
 
 def _find_git_root() -> Path:
     """Walk up from cwd to find .git directory."""
@@ -70,6 +106,15 @@ def run(content: str) -> str:
         )
 
     args = parts[1:] or _DEFAULTS.get(subcommand, [])
+
+    for arg in args:
+        if arg.startswith(_BLOCKED_ARG_PREFIXES):
+            return (
+                f"[error] argument {arg!r} is not allowed: it can write files "
+                f"or execute a program, which would make a read-only "
+                f"subcommand neither."
+            )
+
     full_cmd = ["git", subcommand] + args
     cwd = _find_git_root()
 
@@ -80,6 +125,7 @@ def run(content: str) -> str:
             full_cmd,
             cwd=str(cwd),
             check=False,
+            env=_ENV,
             capture_output=True,
             text=True,
             timeout=_TIMEOUT,
diff --git a/tests/test_shell_git.py b/tests/test_shell_git.py
index 3ed2ff2..af9f2e2 100644
--- a/tests/test_shell_git.py
+++ b/tests/test_shell_git.py
@@ -1,5 +1,7 @@
 """Tests for forge.tools.shell and forge.tools.git."""
 
+from pathlib import Path
+
 import forge.config as cfg
 import forge.tools.git as git_mod
 import forge.tools.shell as shell_mod
@@ -98,3 +100,39 @@ def test_git_status_runs():
 def test_git_log_runs():
     r = git_mod.run("log")
     assert isinstance(r, str) and len(r) > 0
+
+
+def test_git_stash_is_rejected():
+    """
+    `git stash` (bare) is `git stash push`: it removes uncommitted
+    changes from the working tree. It sat in a list labelled
+    "read-only" (audit E-3).
+    """
+    r = git_mod.run("stash")
+    assert "not allowed" in r
+
+
+def test_git_stash_clear_is_rejected():
+    r = git_mod.run("stash clear")
+    assert "not allowed" in r
+
+
+def test_git_rejects_output_argument():
+    r = git_mod.run("log --output=/tmp/forge-git-should-not-exist")
+    assert "[error]" in r
+    assert not Path("/tmp/forge-git-should-not-exist").exists()
+
+
+def test_git_rejects_exec_argument():
+    r = git_mod.run("log --exec=/bin/sh")
+    assert "[error]" in r
+
+
+def test_git_subprocess_env_carries_no_host_secrets(monkeypatch):
+    """
+    git.py used to pass no env at all, so the subprocess inherited
+    everything the API process held -- API_TOKEN included.
+    """
+    assert "API_TOKEN" not in git_mod._ENV
+    assert "OPENROUTER_API_KEY" not in git_mod._ENV
+    assert git_mod._ENV["GIT_TERMINAL_PROMPT"] == "0"

From 59057c11cb024186d0c700f7ebc01df2cd1e1491 Mon Sep 17 00:00:00 2001
From: Kurtisone <104103601+Kurtisone@users.noreply.github.com>
Date: Wed, 12 Aug 2026 18:40:03 +0000
Subject: [PATCH 5/5] docs(deploy): add example compose stack with a real build
 key (audit follow-up)

The root cause behind an entire patch cycle serving stale UI: the
deployment compose lived outside the repo, referenced
`image: forge-core` with no `build:` key, so `podman compose build`
answered "No services to build" and never rebuilt the image. The
container kept serving frozen HTML while the source was patched.

This versioned example lives at deploy/compose.example.yaml with
`build: context: .` for the forge service, which only resolves
correctly when read from the repo root -- the whole point of
versioning it here rather than keeping a fragile relative context in
an out-of-repo compose. The other three services (forge-embedding,
forge-llm built separately; searxng upstream) stay image-only, and the
sysadmin bind mounts are commented with a pointer to deploy/README.md
so the example runs without the host proxies in place. Machine-specific
paths and secrets are placeholders.
---
 deploy/compose.example.yaml | 96 +++++++++++++++++++++++++++++++++++++
 1 file changed, 96 insertions(+)
 create mode 100644 deploy/compose.example.yaml

diff --git a/deploy/compose.example.yaml b/deploy/compose.example.yaml
new file mode 100644
index 0000000..6cdb78d
--- /dev/null
+++ b/deploy/compose.example.yaml
@@ -0,0 +1,96 @@
+# Exemple de stack Forge complète — à COPIER et adapter, pas à lancer tel quel.
+#
+#   cp deploy/compose.example.yaml compose.yaml   # à la racine du repo
+#   # puis adapter les chemins ${HOME}/... et décommenter les montages
+#   # sysadmin une fois les proxies hôte en place (voir deploy/README.md)
+#
+#   podman compose build forge
+#   podman compose up -d
+#
+# Pourquoi cet exemple vit DANS le dépôt, à la racine :
+#   `build: context: .` ne pointe correctement vers le Containerfile que
+#   si le compose est lu depuis la racine du repo. Un compose gardé hors
+#   dépôt avec un `context:` relatif fragile est précisément ce qui a fait
+#   servir une image `forge-core` gelée (ancien code) pendant tout un cycle
+#   de patch — `podman compose build` répondait "No services to build"
+#   faute de clé `build:`, donc l'image n'était jamais reconstruite.
+#
+# Seul `forge` a une clé `build:` : c'est le seul service dont le source
+# vit dans ce dépôt (le Containerfile à la racine). `forge-embedding` et
+# `forge-llm` sont des images llama.cpp construites séparément, et
+# `searxng` est upstream — elles restent en `image:` sans build.
+
+services:
+  searxng:
+    image: docker.io/searxng/searxng:latest
+    container_name: searxng
+    restart: unless-stopped
+    ports:
+      - "8888:8080"
+    volumes:
+      # SearXNG a besoin de "json" dans search.formats (voir config.py,
+      # section web_search) — désactivé par défaut en amont.
+      - ${HOME}/searxng-config/settings.yml:/etc/searxng/settings.yml:ro
+
+  forge-embedding:
+    image: forge-embedding # image llama.cpp embedding, construite séparément
+    container_name: forge-embedding
+    restart: unless-stopped
+    ports:
+      - "8082:8081"
+    volumes:
+      - ${HOME}/path/to/forge-embedding/models/:/models:Z
+
+  forge-llm:
+    image: forge-llm # image llama.cpp, construite séparément
+    container_name: forge-llm
+    restart: unless-stopped
+    devices:
+      - /dev/dri:/dev/dri # accès GPU (iGPU du Steam Deck)
+    annotations:
+      run.oci.keep_original_groups: "1"
+    ports:
+      - "8080:8080"
+    volumes:
+      - ${HOME}/path/to/forge-llm/models:/models
+    command:
+      - -m
+      - /models/VOTRE-MODELE.Q4_K_M.gguf
+      - --host
+      - 0.0.0.0
+      - --port
+      - "8080"
+      - -c
+      - "8192"
+      - -ngl
+      - "28"
+      - -t
+      - "8"
+
+  forge:
+    # La clé build est ce qui manquait : sans elle, compose ne reconstruit
+    # jamais l'image et `build` répond "No services to build".
+    build:
+      context: .
+      containerfile: Containerfile
+    image: forge-core
+    container_name: forge
+    restart: unless-stopped
+    annotations:
+      run.oci.keep_original_groups: "1"
+    ports:
+      - "8000:8000"
+    env_file:
+      # Copier depuis .env.example ; jamais commité. Doit contenir
+      # API_TOKEN (sinon Forge refuse de démarrer — voir audit C-1).
+      - .env.local
+    volumes:
+      - ./data:/app/data
+      # Montages sysadmin (v3.11) — à décommenter UNE FOIS les proxies
+      # hôte en place (journal en lecture seule, proxy D-Bus filtré,
+      # proxy podman read-only). Détails et mise en place complète :
+      # deploy/README.md. Les laisser actifs sans les proxies fait
+      # échouer le démarrage du container sur des chemins absents.
+      # - /var/log/journal:/host-journal:ro
+      # - ${XDG_RUNTIME_DIR}/forge-dbus-proxy:/run/forge-dbus-proxy:ro
+      # - ${XDG_RUNTIME_DIR}/forge-podman-ro-proxy.sock:/run/forge-podman-ro-proxy.sock:ro