Security audit, batch 3: prompt injection and honest boundaries

.env.example

@@ -48,13 +48,33 @@
 # if you want memory's recall answers to read naturally.
 # MAX_STEPS=1
 
+# Raising MAX_STEPS above 1 is what opens the indirect prompt-injection
+# surface of audit E-2: from the second step on, the previous step's
+# tool output is part of the prompt choosing the next tool call, so the
+# text of a fetched web page or a system log gets a say in it. The
+# orchestrator answers that deterministically -- once a run has called
+# web_fetch/web_search/research/sysadmin, no later step of that run may
+# dispatch shell, test, or files:write. Set this to true only if you
+# have a real flow that needs to write after fetching AND you trust
+# every source it reads. A blocked step is not a dead end: the same
+# request asked as a fresh turn starts with a clean slate.
+# ALLOW_MUTATION_AFTER_EXTERNAL_DATA=false
+
 # --- Tools ----------------------------------------------------------------
 # Only tools listed here are dispatchable, regardless of what a
 # module implements. chat,code is the conservative default; add
 # files/shell/git once you trust the sandboxing for your setup,
 # memory once you have an embedding server running (see below), and
 # review/test/web_fetch/web_search/research as each of their own
 # sections below is configured.
+#
+# One combination is worth knowing about before you write it (audit
+# E-1): `files` + `test` is equivalent to `shell`, whatever
+# SHELL_ALLOWED_COMMANDS says. Running pytest means executing the
+# Python code in the workspace -- that is what a test runner is -- so
+# writing a file and then running it is arbitrary code execution
+# through two tools that each look narrow on their own. A warning is
+# logged at startup when both are enabled. See SECURITY.md.
 # ENABLED_TOOLS=chat,code,files,shell,git,memory,recall,review,test,web_fetch,web_search,research
 # WORKSPACE_DIR=data/workspace
 # SHELL_TIMEOUT=30

README.md

@@ -657,6 +657,19 @@ Same commands locally, after `pip install -r requirements-dev.txt`.
 
 ---
 
+### Security
+
+Forge dispatches model-chosen tools on your own machine, sometimes
+against data it fetched from elsewhere. [SECURITY.md](SECURITY.md)
+states the threat model it is built for (one operator, one machine,
+private network, public repo), what is enforced deterministically in
+code rather than asked of the model, and the limits that are known and
+accepted -- including the one worth reading before you edit
+`ENABLED_TOOLS`: `files` and `test` together are equivalent to
+`shell`.
+
+---
+
 ### Status
 
 Forge is an experimental local runtime, not a production framework.

SECURITY.md

@@ -0,0 +1,118 @@
+# Security
+
+Forge dispatches tools chosen by a language model, on a machine you
+own, against data you don't always control. That combination is the
+whole design, so this document says what it is assumed to protect
+against and what it is not — before the interesting parts of the
+roadmap (an Evolution Runtime that writes its own code) make those
+assumptions much harder to change.
+
+## Threat model
+
+Forge is built for **one operator, one machine, private network
+access**. Concretely:
+
+- A single trusted user. There are no roles, no per-user separation,
+  no audit trail attributable to a person.
+- Reachable over WireGuard or localhost, not from the open internet.
+  `API_TOKEN` is the only thing between a caller and `/chat`, and
+  `/chat` dispatches tools.
+- The **repository is public**; the deployment is not. Nothing in
+  `main` may contain a secret, and configuration lives in
+  `.env.local`, which is not tracked.
+- The host is the operator's own (a Steam Deck today, a home server
+  next). Forge is not a shared service, and hardening it into one
+  would be a different project with a stricter version of this file.
+
+**In scope** — things Forge is expected to resist:
+
+- An unauthenticated caller who can reach the port.
+- A hostile or compromised **web page** that Forge fetches, and a
+  hostile **log line** that Forge reads. These reach the prompt that
+  chooses the next tool.
+- The model itself being wrong, degenerate, or steered. Every
+  protection that matters is enforced in code, not asked of the model.
+
+**Out of scope** — deliberately, not by oversight:
+
+- A malicious operator. Anyone able to edit `.env.local` can enable
+  `shell` with `python3` in its allowlist and has the machine.
+- A compromised host, hypervisor, or LLM backend.
+- The supply chain beyond pinning: `requirements.txt` and the base
+  image are pinned to exact versions and a digest, which makes builds
+  reproducible and unexpected upgrades visible. It does not audit
+  what is in those versions.
+- Denial of service. Rate limiting exists to stop accidental
+  hammering, not a determined attacker.
+
+## What is enforced in code
+
+These are deterministic. None of them depend on the model behaving,
+which matters because prompt wording has failed to steer this model
+three separate recorded times on this project.
+
+| Boundary | Where | What it guarantees |
+| --- | --- | --- |
+| Tool opt-in | `config.ENABLED_TOOLS`, `tools/registry.py` | A module with `run()` is not dispatchable until it is listed. |
+| Auth | `config.API_TOKEN` | Forge refuses to start without a token unless `API_ALLOW_UNAUTHENTICATED=true` is written down explicitly. |
+| Workspace confinement | `tools/files.py` `_safe_path`, `tools/review.py`, `tools/test.py` | Paths resolving outside `WORKSPACE_DIR` are rejected before any filesystem call. |
+| SSRF | `tools/web_fetch.py` | Private, loopback and link-local resolved IPs are blocked. Not configurable, on purpose — Forge sits on a home network. |
+| Escalation guard | `orchestrator.py` | Once a run has called `web_fetch`/`web_search`/`research`/`sysadmin`, no later step of that run may dispatch `shell`, `test`, or `files:write`. |
+| Read-only host access | `deploy/podman_ro_proxy.py`, `deploy/forge-dbus-proxy.sh` | `sysadmin` can list units and read logs. Start/stop/exec are refused at the proxy, before Forge's own code is in a position to decide. |
+| Loop guard | `orchestrator.py` | The same `(tool, content)` pair cannot be dispatched twice in one run. |
+| Non-root container | `Containerfile`, `deploy/compose.example.yaml` | Runs as `forge:forge` (1000:1000). Requires `userns_mode: keep-id` at runtime; a test asserts the two halves stay coupled. |
+
+## Known limits
+
+Stated because a limit you know about is a decision, and one you don't
+is a surprise.
+
+**`files` + `test` is equivalent to `shell`.** Running pytest means
+executing the Python code in the workspace — that is what a test
+runner is. Write a file, then run it. `pytest` also auto-loads
+`conftest.py` before collecting anything, so even an invocation naming
+one innocuous file executes a `conftest.py` next to it. The allowlist
+in `tools/test.py` restricts which binary starts, not what that binary
+then executes; argument confinement bounds where that code may come
+from, not that it runs. A warning is logged at startup when both tools
+are enabled. Fixing this properly needs a disposable container per
+run.
+
+**The escalation guard is per-run, not per-session.** Content fetched
+from a hostile page in one turn can be written to the workspace and
+read back in a later turn, where the run starts untainted. Making the
+taint persist would mean a fetch poisoning every subsequent turn until
+something cleared it, with no obvious moment to clear it. Per-run is
+the deliberate trade, not an oversight.
+
+**`files:read` does not taint a run.** Read-then-write is the one
+legitimate multi-step flow this project uses. Treating a workspace
+read as external ingest would break it to defend against the
+operator's own files.
+
+**Prompt-level defences are nudges.** The provenance markers around
+tool output in `router/prompt.py` ask the model not to be steered by
+what it reads. They are not what anything rests on; the escalation
+guard is.
+
+**`MAX_STEPS=1` is doing real work.** At the default, there is never a
+second routing decision, so tool output never reaches a prompt that
+chooses a tool. Raising it is what opens that surface, and what the
+escalation guard exists for.
+
+**In-memory rate limiting is single-process.** Multiple workers each
+keep their own window.
+
+**`sysadmin` can be confidently wrong.** It reads real logs and asks a
+local model to explain them. Its output is a proposal for a human,
+never applied automatically — by design, no command in
+`graphs/sysadmin.py` can mutate anything, and that is fixed in code
+rather than configurable.
+
+## Reporting a vulnerability
+
+Open a GitHub issue on
+[Kurtisone/forge](https://github.com/Kurtisone/forge/issues), or use
+GitHub's private vulnerability reporting for anything you would rather
+not post publicly. This is a personal project maintained by one
+person: expect a best-effort response, not an SLA.

src/forge/config.py

@@ -57,6 +57,25 @@ def _bool(name: str, default: str = "false") -> bool:
 # see Orchestrator.run() -- so this ceiling exists to make sure that,
 # whatever a model decides, a run can't loop forever.
 MAX_STEPS = int(os.getenv("MAX_STEPS", "1"))
+# Raising MAX_STEPS above 1 is what opens the indirect prompt-injection
+# surface described in audit E-2: from the second step onward, the
+# previous step's tool output is part of the prompt that decides the
+# next tool call, so the text of a web page or a log influences that
+# decision. The guard in orchestrator.py answers that deterministically
+# -- once a step has pulled in data Forge doesn't control (web_fetch,
+# web_search, research, sysadmin), no later step in the same run may
+# dispatch a mutating tool (shell, test, files:write).
+#
+# Set this to true only if you have a real multi-step flow that needs
+# to write after fetching, and you trust every source it reads. It
+# costs you the one guarantee that doesn't depend on the model
+# behaving: prompt wording has failed three times on this project, a
+# refusal in orchestrator.py cannot be talked out of. A blocked step
+# is not a dead end either -- the same request asked again as a fresh
+# turn starts with a clean slate.
+ALLOW_MUTATION_AFTER_EXTERNAL_DATA = _bool(
+    "ALLOW_MUTATION_AFTER_EXTERNAL_DATA", "false"
+)
 
 # --- Memory --------------------------------------------------------------
 MEMORY_ENABLED = _bool("MEMORY_ENABLED", "true")

src/forge/orchestrator.py

@@ -17,10 +17,19 @@
    other than a non-empty str is a ToolExecutionError.
 6. Execution is traceable: AgentState accumulates a TraceStep per
    step and trace.save() writes it to disk when TRACE_ENABLED=true.
+7. No tool escalation after external data: once a step has pulled in
+   content Forge doesn't control, no later step of the same run may
+   dispatch a mutating tool. See _EXTERNAL_INGEST_TOOLS below.
 """
 
+import json
+
 from forge import memory, subtrace, trace
-from forge.config import MAX_STEPS, MEMORY_ENABLED
+from forge.config import (
+    ALLOW_MUTATION_AFTER_EXTERNAL_DATA,
+    MAX_STEPS,
+    MEMORY_ENABLED,
+)
 from forge.errors import LoopGuardError, ProviderError, ToolExecutionError
 from forge.llm import call_llm
 from forge.logger import log
@@ -30,6 +39,70 @@
 
 load_tools()
 
+# --- Escalation guard (audit E-2) -------------------------------------
+#
+# Tools whose output is content Forge does not control: a web page, a
+# search result page, a system log. From the second step onward that
+# text sits in the prompt that picks the next tool (see step_context
+# in run() below and router/prompt.py), so it is in a position to
+# suggest one.
+#
+# files:read is deliberately NOT here. It reads inside WORKSPACE_DIR,
+# and the read-then-write chain is a designed flow (v3.9: "remplace X
+# par Y dans hello.go" reads the real file, then writes it back with
+# the change applied). Taking files:read as tainting would break the
+# one legitimate multi-step flow this project actually uses, to
+# defend against a file the user's own workspace put there -- the
+# wrong trade. Nothing stops a hostile page's content being written
+# to the workspace in one turn and read back in another; that is a
+# real limit of a per-run taint and it is stated in SECURITY.md
+# rather than papered over here.
+_EXTERNAL_INGEST_TOOLS = frozenset({"web_fetch", "web_search", "research", "sysadmin"})
+
+# Tools that change something outside the run. "code" is not one: it
+# returns source as text, it doesn't execute or persist anything.
+_MUTATING_TOOLS = frozenset({"shell", "test"})
+
+# files is both, depending on its action -- resolved by _is_mutating().
+_FILES_READONLY_ACTIONS = frozenset({"read", "list"})
+
+if ALLOW_MUTATION_AFTER_EXTERNAL_DATA:
+    # Logged once at import, same reasoning as shell.py's allowlist
+    # tripwire: a configuration fact belongs in the startup log, not
+    # repeated into the output of every run where people learn to
+    # scroll past it.
+    log.warning(
+        "orchestrator: ALLOW_MUTATION_AFTER_EXTERNAL_DATA is set -- a run "
+        "that has fetched a web page or read system logs may go on to "
+        "write files or run commands in a later step. The content of "
+        "that page is part of the prompt choosing that step."
+    )
+
+
+def _is_mutating(tool: str, content: str) -> bool:
+    """
+    Does dispatching this call change anything outside the run?
+
+    Fails closed for files: anything that isn't demonstrably a read or
+    a list counts as a write. A malformed payload is exactly the case
+    where guessing "probably harmless" is worst -- files.py's own
+    parser is more forgiving than this check, and the gap between the
+    two is where an escalation would live.
+    """
+    if tool in _MUTATING_TOOLS:
+        return True
+    if tool != "files":
+        return False
+
+    try:
+        payload = json.loads(content)
+    except (ValueError, TypeError):
+        return True
+    if not isinstance(payload, dict):
+        return True
+    action = str(payload.get("action", "")).strip().lower()
+    return action not in _FILES_READONLY_ACTIONS
+
 
 class Orchestrator:
     def __init__(self, max_steps: int = MAX_STEPS):
@@ -118,10 +191,45 @@ def run(self, user_input: str) -> AgentResult:
                 return self._finish(state, remember=False)
             state.seen_calls.add(call_signature)
 
+            # --- Escalation guard ----------------------------------------
+            # Checked before dispatch, never after: the point is that
+            # the tool must not run, not that its effect gets reported.
+            if (
+                state.external_data_seen
+                and not ALLOW_MUTATION_AFTER_EXTERNAL_DATA
+                and _is_mutating(decision.tool, decision.content)
+            ):
+                note = (
+                    f"escalation guard: tool={decision.tool!r} would mutate "
+                    f"after {state.external_data_source!r} pulled in external "
+                    "data earlier in this run"
+                )
+                log.error(note)
+                ts.abandon(note)
+                state.ok = False
+                state.error = note
+                state.final_output = (
+                    "Stopped: this run already read outside data "
+                    f"(via {state.external_data_source}), so it can no longer "
+                    "write files or run commands. If that was what you "
+                    "wanted, ask again as a separate request."
+                )
+                state.final_tool = decision.tool
+                return self._finish(state, remember=False)
+
             # --- Dispatch ------------------------------------------------
             result = self._dispatch(decision.tool, decision.content)
             ts.finish(result)
 
+            if decision.tool in _EXTERNAL_INGEST_TOOLS:
+                # Marked on dispatch, not on success: a tool that
+                # failed mid-way may still have put part of what it
+                # read into the trace and the logs, and "it errored so
+                # nothing came in" is an assumption about code this
+                # module deliberately doesn't look inside.
+                state.external_data_seen = True
+                state.external_data_source = decision.tool
+
             state.final_output = result.output
             state.final_tool = result.tool
             state.ok = result.ok