fix(security): remediate 2026-07-12 re-audit findings

.github/workflows/pr.yml

@@ -750,7 +750,12 @@ jobs:
     # was cleared by infra_smoke_reseed_runtime_budget (demo-ubi CI-excluded
     # in ui/playwright.config.ts testIgnore). `needs: [docker, docker-ui]`
     # below stays consistent — when this skips, the images simply go unused.
-    if: ${{ vars.SMOKE_TEST == 'true' && vars.SKIP_HEAVY_CI != 'true' }}
+    # The final clause is a FORK-PR GUARD (security audit 2026-07-12): this job
+    # mounts secrets.OPENAI_API_KEY_TEST and then runs the PR head's checked-out
+    # code (make up / install.sh / app). Without this guard, a fork PR's
+    # untrusted code could exfiltrate the key the moment a maintainer approves
+    # its CI run while SMOKE_TEST is enabled. Same-repo branch PRs still run it.
+    if: ${{ vars.SMOKE_TEST == 'true' && vars.SKIP_HEAVY_CI != 'true' && github.event.pull_request.head.repo.full_name == github.repository }}
     runs-on: ubuntu-24.04
     # Bumped 15 → 25 min as part of infra_solr_smoke_stability PR #383.
     # Post infra_smoke_reseed_runtime_budget (demo-ubi CI-excluded in

backend/app/adapters/solr.py

@@ -1092,6 +1092,16 @@ def render(
         the Jinja sandbox forbids attribute access so unified params are
         flat (``field_boosts``, not ``boost_config.fields``).
         """
+        # Neutralize a leading Solr local-params block ({!parser ...}) in the
+        # untrusted query_text BEFORE it is interpolated (security audit
+        # 2026-07-12 F1 — broadened from the 2026-07-11 q-only fix). Escaping it
+        # at the source means query_text is safe wherever a benign template
+        # author places it (q, fq, bf, boost, pf, ...) — not just `q` — while
+        # leaving the author's own static local params in the template body
+        # untouched. Solr only honors the block at the very start of a param
+        # value, so escaping the leading `{` in the user text is sufficient.
+        query_text = _neutralize_leading_local_params(query_text)
+
         # Shared pre-render hook (normalizer pop + declared-param check + Jinja
         # render) lives in adapters.render; it runs BEFORE the Solr-specific LTR
         # pre-flight + _pivot_to_solr_params steps below.
@@ -1110,13 +1120,10 @@ def render(
             self._check_ltr_model_available(rerank)
 
         body = self._pivot_to_solr_params(rendered)
-        # Neutralize a leading local-params block on the user-controlled `q`
-        # (security audit 2026-07-11 finding #6). Only the render path (user
-        # query_text) reaches here; the operator run_query passthrough bypasses
-        # render, and system-generated `rq` local params are left untouched.
-        q_val = body.get("q")
-        if isinstance(q_val, str):
-            body["q"] = _neutralize_leading_local_params(q_val)
+        # query_text was neutralized at the top of render() (above), so any
+        # param that interpolated it (q, fq, bf, boost, pf, ...) already carries
+        # the escaped form. The operator run_query passthrough bypasses render
+        # entirely, and system-generated `rq` (LTR) local params are legitimate.
         return NativeQuery(query_id=template.name, body=body)
 
     def _check_ltr_model_available(self, rerank: dict[str, Any]) -> None:

backend/app/agent/confirmation.py

@@ -113,6 +113,21 @@
 #: contains "can't" / "couldn't").
 _NEGATION_CONTRACTIONS: tuple[str, ...] = ("can't", "couldn't")
 
+#: Fold unicode apostrophe / prime variants to the ASCII apostrophe U+0027 so
+#: the contraction check above can't be bypassed by smart-punctuation input
+#: (iOS/macOS keyboards emit U+2019 by default — "can’t but yes" would
+#: otherwise slip past as affirmative). Security audit 2026-07-12 F3.
+_APOSTROPHE_FOLD = str.maketrans(
+    {
+        "’": "'",  # ’ right single quote (smart apostrophe)
+        "‘": "'",  # ‘ left single quote
+        "ʼ": "'",  # ʼ modifier letter apostrophe
+        "ʹ": "'",  # ʹ modifier letter prime
+        "′": "'",  # ′ prime
+        "＇": "'",  # ＇ fullwidth apostrophe
+    }
+)
+
 
 def is_affirmative(text: str) -> bool:
     """Return ``True`` if ``text`` reads as user affirmation of a mutating action.
@@ -126,7 +141,7 @@ def is_affirmative(text: str) -> bool:
     """
     if not text:
         return False
-    lowered = text.lower()
+    lowered = text.lower().translate(_APOSTROPHE_FOLD)
     if any(neg in lowered for neg in _NEGATION_CONTRACTIONS):
         return False
     words = set(re.findall(r"[a-z]+", lowered))

backend/app/agent/orchestrator.py

@@ -47,6 +47,7 @@
 from pydantic import ValidationError
 
 from backend.app.agent.confirmation import (
+    _APOSTROPHE_FOLD,
     MUTATING_TOOL_NAMES,
     is_affirmative,
 )
@@ -156,7 +157,10 @@ def _is_authorized_mutation(
     """
     if not last_assistant_text:
         return False
-    assistant_lower = last_assistant_text.lower()
+    # Fold unicode apostrophes so both the tool-name match and the negation
+    # check (which use ASCII forms like `won'?t`) can't be bypassed by smart
+    # quotes (Gemini review — same class as F3).
+    assistant_lower = last_assistant_text.lower().translate(_APOSTROPHE_FOLD)
     # Collect the MUTATING_TOOL_NAMES that appear as whole words in the assistant
     # turn. 0 matches: assistant didn't propose this tool. 2+: ambiguous — one
     # affirmative cannot bind to a specific tool, so reject all and let the model
@@ -169,9 +173,40 @@ def _is_authorized_mutation(
     ]
     if len(matching_tools) != 1 or matching_tools[0] != tool_name:
         return False
+    # Reject when the assistant NEGATED the tool rather than proposing it
+    # (security audit 2026-07-12 F2 — "I will not cancel_study" + "yes" must not
+    # authorize). Conservative 3-word window so a distant negation in unrelated
+    # prose ("this won't take long — shall I cancel_study?") does not
+    # false-reject a genuine proposal.
+    if _tool_mention_is_negated(assistant_lower, tool_name):
+        return False
     return is_affirmative(last_user_text)
 
 
+#: Negation tokens that, when they precede a mutating tool name within a few
+#: words, indicate the assistant is declining/hypothesizing rather than
+#: proposing the tool. Used by :func:`_tool_mention_is_negated` (F2).
+_NEG_BEFORE_TOOL = (
+    r"\b(?:not|never|won'?t|can'?t|cannot|do(?:es)?n'?t|would\s*n'?t|"
+    r"instead\s+of|rather\s+than|no\s+need\s+to)\b"
+)
+
+
+@functools.lru_cache(maxsize=64)
+def _negated_tool_pattern(tool_name: str) -> re.Pattern[str]:
+    """Build a regex matching ``<negation> …(≤3 words)… <tool_name>``.
+
+    Matches both the underscore and spaced forms of the tool name.
+    """
+    forms = "|".join(re.escape(f) for f in (tool_name, tool_name.replace("_", " ")))
+    return re.compile(_NEG_BEFORE_TOOL + r"(?:\s+\w+){0,3}\s+(?:" + forms + r")\b", re.IGNORECASE)
+
+
+def _tool_mention_is_negated(assistant_lower: str, tool_name: str) -> bool:
+    """True if ``tool_name`` is preceded (within 3 words) by a negation."""
+    return _negated_tool_pattern(tool_name).search(assistant_lower) is not None
+
+
 def _build_tool_error_events(
     *,
     tool_call_id: str,
@@ -228,6 +263,14 @@ async def run_turn(
     iterations = 0
     total_tokens = 0
     total_cost = 0.0
+    # Security audit 2026-07-12 F1: at most ONE mutating tool may be dispatched
+    # per turn (per user affirmative). Without this, a single "yes" (which binds
+    # to a tool *name*, not its arguments) authorizes unlimited invocations of
+    # that tool against arbitrary targets across the tool loop — an
+    # injection→mutation amplifier ("open PRs for ALL pending proposals" after
+    # the human confirmed one). Once a mutation fires, further mutating calls in
+    # this turn are refused; the model must re-propose in a fresh turn.
+    mutating_dispatched = False
 
     while iterations < MAX_LOOP_ITERATIONS:
         iterations += 1
@@ -406,24 +449,43 @@ async def run_turn(
                 continue
 
             # Confirmation guard for mutating tools.
-            if tc_name in MUTATING_TOOL_NAMES and not _is_authorized_mutation(
-                tool_name=tc_name,
-                last_assistant_text=last_assistant_text,
-                last_user_text=last_user_text,
-            ):
-                for ev in _build_tool_error_events(
-                    tool_call_id=tc_id,
+            if tc_name in MUTATING_TOOL_NAMES:
+                if mutating_dispatched:
+                    # F1: one mutation per confirmation — a mutating tool already
+                    # ran this turn, so this additional mutating call is not
+                    # covered by the user's single affirmative. Refuse and force
+                    # a fresh propose-and-confirm.
+                    for ev in _build_tool_error_events(
+                        tool_call_id=tc_id,
+                        tool_name=tc_name,
+                        error_code="confirmation_required",
+                        detail=(
+                            f"Confirmation required for {tc_name}. Only one mutating "
+                            "action may run per confirmation; propose this action "
+                            "again so the user can confirm it specifically."
+                        ),
+                        history=history,
+                    ):
+                        yield ev
+                    continue
+                if not _is_authorized_mutation(
                     tool_name=tc_name,
-                    error_code="confirmation_required",
-                    detail=(
-                        f"Confirmation required for {tc_name}. The assistant must "
-                        "explicitly propose this tool, and the user must affirmatively "
-                        "confirm, before dispatch."
-                    ),
-                    history=history,
+                    last_assistant_text=last_assistant_text,
+                    last_user_text=last_user_text,
                 ):
-                    yield ev
-                continue
+                    for ev in _build_tool_error_events(
+                        tool_call_id=tc_id,
+                        tool_name=tc_name,
+                        error_code="confirmation_required",
+                        detail=(
+                            f"Confirmation required for {tc_name}. The assistant must "
+                            "explicitly propose this tool, and the user must affirmatively "
+                            "confirm, before dispatch."
+                        ),
+                        history=history,
+                    ):
+                        yield ev
+                    continue
 
             # Dispatch the impl.
             impl = TOOL_REGISTRY[tc_name]
@@ -452,6 +514,10 @@ async def run_turn(
                 continue
 
             # Successful dispatch.
+            if tc_name in MUTATING_TOOL_NAMES:
+                # F1: mark that this turn's single mutation is spent; any further
+                # mutating call this turn will be refused above.
+                mutating_dispatched = True
             yield ToolResultEvent(id=tc_id, name=tc_name, result=result)
             yield ToolMessagePersistEvent(tool_call_id=tc_id, content={"result": result})
             history.append(

backend/app/api/v1/schemas/studies.py

@@ -38,7 +38,13 @@
 class CreateQueryTemplateRequest(BaseModel):
     """Request body for ``POST /api/v1/query-templates``."""
 
-    name: str = Field(min_length=1, max_length=256)
+    # Path-safe pattern (security audit 2026-07-12): the name is interpolated
+    # into the params-file path (`{name}.params.json`) by the open_pr worker.
+    # Must start alphanumeric and exclude path separators so a name like
+    # `../.github/workflows/x` can't relocate the written file within the repo
+    # branch (the clone-root containment check bounds the host FS but not the
+    # intended config_path subtree). Allows the usual kebab/snake/dotted names.
+    name: str = Field(min_length=1, max_length=256, pattern=r"^[A-Za-z0-9][A-Za-z0-9 ._-]*$")
     engine_type: EngineTypeWire
     body: str = Field(min_length=1)
     declared_params: dict[str, str] = Field(default_factory=dict)

backend/app/core/cors.py

@@ -0,0 +1,24 @@
+# SPDX-FileCopyrightText: 2026 soundminds.ai
+#
+# SPDX-License-Identifier: Apache-2.0
+
+"""CORS policy helpers.
+
+Kept out of ``main`` so they're importable without constructing ``Settings`` /
+building the FastAPI app.
+"""
+
+from __future__ import annotations
+
+
+def cors_allow_credentials(origins: list[str]) -> bool:
+    """Return whether CORS credentials may be enabled for ``origins``.
+
+    Security audit 2026-07-11 finding #10: a wildcard origin combined with
+    ``allow_credentials=True`` is unsafe — Starlette reflects the request Origin
+    (rather than sending a literal ``*``), which lets ANY site make credentialed
+    cross-origin requests. So credentials are disabled whenever a wildcard is
+    configured; MVP1 has no cookies/auth so nothing depends on credentialed CORS
+    today. Explicit origins keep credentials.
+    """
+    return "*" not in origins

backend/app/domain/git/redaction.py

@@ -50,6 +50,21 @@
 _OPENAI_KEY_PATTERN = re.compile(r"\bsk-[A-Za-z0-9_-]{20,}")
 REDACTED_OPENAI_PLACEHOLDER = "[REDACTED-OPENAI-KEY]"
 
+# Credentials embedded in a connection URI: `scheme://user:password@host`.
+# Redacts the password portion of a DSN — the Postgres password (RelyLoop's
+# one boot-blocking secret) and any `redis://:pass@`, so an asyncpg/redis
+# connection-error log line that echoes the DSN can't leak it (security audit
+# 2026-07-12). Requires the trailing `@` (userinfo) so a plain `host:port/path`
+# URL with no credentials never matches.
+# The password class excludes ``]`` so this pattern never re-redacts an already
+# substituted ``[REDACTED-GH-TOKEN]`` placeholder that sits in the userinfo of a
+# git auth URL (``https://x:ghp_…@github.com`` → GH pattern runs first and wins).
+_DSN_PASSWORD_PATTERN = re.compile(
+    r"([a-z][a-z0-9+.\-]*://[^\s:/@]*:)[^\s@/\]]+(@)",
+    re.IGNORECASE,
+)
+REDACTED_DSN_PLACEHOLDER = r"\1[REDACTED-URL-PASSWORD]\2"
+
 
 def redact_token(text: Any) -> Any:
     """Replace any GitHub PAT / OpenAI key pattern with its placeholder.
@@ -61,7 +76,8 @@ def redact_token(text: Any) -> Any:
     if not isinstance(text, str):
         return text
     redacted = _GH_TOKEN_PATTERN.sub(REDACTED_PLACEHOLDER, text)
-    return _OPENAI_KEY_PATTERN.sub(REDACTED_OPENAI_PLACEHOLDER, redacted)
+    redacted = _OPENAI_KEY_PATTERN.sub(REDACTED_OPENAI_PLACEHOLDER, redacted)
+    return _DSN_PASSWORD_PATTERN.sub(REDACTED_DSN_PLACEHOLDER, redacted)
 
 
 def _redact_value(value: Any) -> Any:

backend/app/domain/study/template_validator.py

@@ -93,18 +93,24 @@ class ReservedParamReferenced(ValueError):
 
 
 def _assert_query_text_is_tojson_escaped(ast: nodes.Template) -> None:
-    """Reject any ``query_text`` reference not wrapped by a ``tojson`` filter.
-
-    A ``query_text`` Name node is considered safe iff it is a descendant of a
-    ``Filter`` node whose name is ``tojson`` (so ``{{ query_text | tojson }}``
-    and ``{{ query_text | trim | tojson }}`` both pass, while
-    ``{{ query_text }}`` and ``{{ query_text | upper }}`` are rejected).
+    r"""Reject any ``query_text`` reference not JSON-escaped as the LAST transform.
+
+    A ``query_text`` Name node is safe iff it sits inside an ``Output`` (``{{ }}``)
+    expression whose **outermost** node is a ``tojson`` filter — so nothing runs
+    after the JSON-escaping. ``{{ query_text | tojson }}`` and
+    ``{{ query_text | trim | tojson }}`` pass; ``{{ query_text }}``,
+    ``{{ query_text | upper }}``, and ``{{ query_text | tojson | replace('\"','') }}``
+    are rejected. The old check accepted *any* ancestor ``tojson`` filter, so a
+    filter chained AFTER ``tojson`` could strip the escaping and re-open the
+    injection (security audit 2026-07-12 F-tojson-outermost).
     """
     covered: set[int] = set()
-    for filt in ast.find_all(nodes.Filter):
-        if filt.name == "tojson":
-            for name_node in filt.find_all(nodes.Name):
-                covered.add(id(name_node))
+    for output in ast.find_all(nodes.Output):
+        for expr in output.nodes:
+            if isinstance(expr, nodes.Filter) and expr.name == "tojson":
+                for name_node in expr.find_all(nodes.Name):
+                    if name_node.name == "query_text":
+                        covered.add(id(name_node))
 
     for name_node in ast.find_all(nodes.Name):
         if name_node.name == "query_text" and id(name_node) not in covered:

backend/app/main.py

@@ -48,6 +48,7 @@
 from backend.app.api.v1 import query_templates as query_templates_router
 from backend.app.api.v1 import studies as studies_router
 from backend.app.api.webhooks import github as webhook_github_router
+from backend.app.core.cors import cors_allow_credentials
 from backend.app.core.logging import configure_logging, get_logger
 from backend.app.core.settings import get_settings
 from backend.app.db.session import get_session_factory
@@ -221,13 +222,8 @@ async def lifespan(_app: FastAPI) -> AsyncIterator[None]:
 # (comma-separated). Empty string disables. MVP1 default covers the local Next
 # dev server; operators add production origins at MVP3.
 _cors_origins = [o.strip() for o in get_settings().cors_allow_origins.split(",") if o.strip()]
-# Security audit 2026-07-11 finding #10: a wildcard origin combined with
-# allow_credentials=True is unsafe — Starlette reflects the request Origin
-# (rather than sending a literal "*"), which lets ANY site make credentialed
-# cross-origin requests. Disable credentials whenever a wildcard is configured;
-# MVP1 has no cookies/auth so nothing depends on credentialed CORS today.
 _cors_has_wildcard = "*" in _cors_origins
-_cors_allow_credentials = not _cors_has_wildcard
+_cors_allow_credentials = cors_allow_credentials(_cors_origins)
 if _cors_has_wildcard:
     logger.warning(
         "CORS_ALLOW_ORIGINS contains '*'; disabling allow_credentials to avoid "

backend/app/services/agent_judgments_dispatch.py

@@ -52,6 +52,7 @@
 from backend.app.llm.capability_check import read_or_recompute_capability_result
 from backend.app.llm.cost_model import known_models
 from backend.app.services.cluster import build_adapter
+from backend.app.services.cluster_url_policy import assert_base_url_allowed
 from backend.app.services.ubi_errors import UbiNotEnabledError
 from backend.app.services.ubi_reader import UbiReader
 from backend.app.services.ubi_readiness import count_ubi_events_in_window
@@ -508,6 +509,10 @@ async def start_ubi_judgment_generation(
     # §8.5 503 CLUSTER_UNREACHABLE envelope rather than bubbling as an
     # unstructured 500 (GPT-5.5 PR #317 final-review finding #2). The
     # UbiNotEnabledError → 412 catch is narrower and runs first.
+    # SSRF re-validation on this reuse path (security audit 2026-07-12): the
+    # registration-time guard is not enough — re-check on every connection so a
+    # rebound host is caught here. No-op unless RELYLOOP_ALLOW_PRIVATE_CLUSTERS=False.
+    await assert_base_url_allowed(cluster.base_url)
     adapter = build_adapter(cluster)
     try:
         # Inject Settings-backed scan ceilings (FR-5/FR-7) so the dispatcher's

backend/app/services/cluster.py

@@ -548,6 +548,13 @@ def build_adapter(cluster: Cluster) -> ClusterAdapter:
     ``infra_adapter_solr`` Story A1). The unified ``SearchAdapter`` Protocol
     contract means callers don't care which concrete class is returned —
     they call Protocol methods.
+
+    **SSRF note:** any code path that will make an outbound request to the
+    cluster must ``await assert_base_url_allowed(cluster.base_url)`` immediately
+    before calling this (the async workers do — trials/baseline/judgments/
+    judgments_ubi + agent dispatch), so the DNS-rebinding re-validation fires on
+    every connection, not just at registration. This builder itself is sync and
+    does not re-check.
     """
     return _build_adapter_from_args(
         cluster_id=cluster.id,