Merge pull request #1 from ToxMCP/codex/oqt-v0.2.0-cleanup-public

Release v0.3.1: Audit remediation for OQT-01/02/03/04/05

Author: senseibelbi

AGENTS.md

@@ -0,0 +1,56 @@
+# Agent Guidance for O-QT MCP Server
+
+## Scope
+This file applies to the `o-qt-mcp-server-public` repository (canonical target).
+
+## Critical Controls (Added 2026-04-16)
+
+### 1. Applicability Domain (AD) Gating
+- **Files:** `src/tools/implementations/o_qt_qsar_tools.py`, `src/tools/implementations/toolbox_execution.py`, `src/tools/implementations/workflow_runner.py`
+- **Behavior:** `run_qsar_prediction` and `run_qsar_model` now inspect the domain result from the QSAR Toolbox. If the domain status is `"OutOfDomain"`, the result includes:
+  - `"ad_status": "out_of_domain"`
+  - `"ad_warning": true`
+  - `"ad_recommendation": "..."`
+- **Rule:** Do NOT remove these fields. The workflow runner surfaces AD warnings in the Markdown summary.
+
+### 2. Human Review Checkpoints (OQT-02)
+- **Files:** `src/utils/review.py`, `src/tools/implementations/workflow_runner.py`, `config/tool_permissions.default.json`
+- **Behavior:** When `require_human_review=true` is passed to `run_oqt_multiagent_workflow`, the workflow creates up to three checkpoints:
+  1. `chemical_identity` — after resolving the input identifier to a Toolbox record
+  2. `ad_assessment` — when any QSAR prediction reports `ad_warning=true`
+  3. `final_report` — before generating the PDF artifact
+- If checkpoints are pending, the workflow returns `status: "review_required"` with `workflow_id` and `review_checkpoints`. No PDF is generated.
+- Clients can approve/reject checkpoints via the `approve_workflow_checkpoint` tool, then resume by passing the same `workflow_id` (and optionally `checkpoint_approvals`) to the workflow.
+- **Rule:** Do NOT auto-generate artifacts when `require_human_review=true` and checkpoints are pending. Do NOT skip the `ad_assessment` checkpoint for out-of-domain predictions.
+
+### 3. LLM Prompt-Boundary Sanitization
+- **File:** `src/utils/sanitization.py`, `src/integrations/oqt_assistant.py`
+- **Behavior:** All user-supplied identifiers and context strings are sanitized with `sanitize_for_llm()` before entering the oqt_assistant LLM pipeline.
+- **Rule:** If you add new LLM-facing inputs, pipe them through `sanitize_for_llm()`.
+
+### 4. Privacy-Aware Audit Logging
+- **Files:** `src/utils/privacy.py`, `src/tools/registry.py`, `src/api/server.py`, `src/utils/logging.py`
+- **Behavior:**
+  - Audit events hash SMILES, CAS numbers, chemical names, and API keys before logging.
+  - The HTTP audit middleware parses query strings into dictionaries so parameter keys remain readable while values are hashed.
+  - The `PrivacyLogFilter` scrubs SMILES/CAS patterns from free-text log messages and URL query parameters, and hashes whole-value identifiers in structured log extra fields.
+- **Rule:** Do NOT log raw chemical identifiers or secrets. Use `scrub_dict()` on params before audit emit.
+
+### 5. Fallback PDF Provenance
+- **File:** `src/utils/pdf_generator.py`
+- **Behavior:** The fallback PDF includes:
+  - A prominent disclaimer on the first page
+  - An "Applicability Domain Warnings" section when out-of-domain predictions are present
+  - A "Provenance" section showing model count and AD status
+- **Rule:** Keep the disclaimer visible. Do not remove the AD-warning block.
+
+### 6. Search Defaults
+- **File:** `src/tools/implementations/workflow_runner.py`, `src/tools/implementations/o_qt_qsar_tools.py`
+- **Behavior:** `search_type` default is now `"name"` instead of `"auto"` to reduce silent wrong-chemical resolution.
+- **Rule:** Do not revert the default to `"auto"` without explicit user confirmation logic.
+
+## Testing Expectations
+- Any change to AD logic must pass `test_run_qsar_prediction_ad_warning_out_of_domain`.
+- Any change to privacy logic must pass `tests/utils/test_privacy.py`.
+- Any change to sanitization must pass `tests/utils/test_sanitization.py`.
+- Any change to PDF generation must pass `test_generate_pdf_report_includes_disclaimer_and_ad_warnings`.

CHANGELOG.md

@@ -18,6 +18,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ---
 
+## [0.3.1] - 2026-04-17
+
+### Added
+- **Human review checkpoints** (`OQT-02`). When `require_human_review=true` is passed to `run_oqt_multiagent_workflow`, the workflow now pauses at up to three checkpoints (`chemical_identity`, `ad_assessment`, `final_report`) and returns `status: "review_required"` instead of auto-generating artifacts.
+- `approve_workflow_checkpoint` tool to explicitly approve or reject pending workflow checkpoints.
+- **Applicability-domain hard gating** (`OQT-01`). Out-of-domain predictions block PDF generation when `require_human_review=true`.
+- `PrivacyLogFilter` (`OQT-05`) to scrub SMILES, CAS numbers, and chemical names from free-text log messages and URL query parameters before emission.
+- `sanitize_for_llm()` utility (`OQT-04`) to strip control characters, backticks, and dollar signs from untrusted identifiers before they enter LLM-facing contexts.
+- Fallback PDF provenance enhancements (`OQT-03`): disclaimer header, applicability-domain warnings section, and provenance summary showing models run and AD warning count.
+- `qsar_models_executed` field in the workflow response for full QSAR transparency (`MD-004`).
+- Safer search defaults (`HG-001`): `search_type` now defaults to `"name"` instead of `"auto"`.
+- Unit tests for sanitization, privacy scrubbing, and review checkpoint orchestration.
+
+### Changed
+- HTTP audit middleware now parses query strings into dictionaries so parameter keys remain readable while values are hashed.
+- Updated `AGENTS.md` with critical-control policies to prevent future regressions of AD gating, sanitization, privacy, and review defaults.
+
+### Fixed
+- Removed raw chemical identifiers from audit logs; all SMILES, CAS, and chemical-name values are now hashed before emission.
+
+---
+
 ## [0.3.0] - 2026-04-08
 
 ### Added
@@ -60,7 +82,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Dockerfile and docker-compose stack for local development with Toolbox API stub.
 - Taskmaster backlog, documentation set (`docs/auth_testing.md`, `docs/observability.md`, `docs/testing.md`), and CI workflow skeleton.
 
-[Unreleased]: https://github.com/ToxMCP/oqt-mcp/compare/v0.3.0...HEAD
+[Unreleased]: https://github.com/ToxMCP/oqt-mcp/compare/v0.3.1...HEAD
+[0.3.1]: https://github.com/ToxMCP/oqt-mcp/releases/tag/v0.3.1
 [0.3.0]: https://github.com/ToxMCP/oqt-mcp/releases/tag/v0.3.0
 [0.2.0]: https://github.com/ToxMCP/oqt-mcp/releases/tag/v0.2.0
 [0.1.0]: https://github.com/ToxMCP/oqt-mcp/releases/tag/v0.1.0

README.md

@@ -271,6 +271,9 @@ See [docs/auth_testing.md](docs/auth_testing.md) for token generation tips and b
 | `llm_provider` | string | – | Override LLM provider (e.g., `openai`, `openrouter`). |
 | `llm_model` | string | – | LLM model identifier. |
 | `llm_api_key` | string | – | API key when not provided via environment. |
+| `require_human_review` | boolean | – | When `true`, high-risk checkpoints require explicit approval before artifacts are generated. |
+| `workflow_id` | string | – | Optional workflow ID for resuming a review-paused workflow. |
+| `checkpoint_approvals` | array[{checkpoint_id, decision, comments}] | – | Pre-approved checkpoints to resume a paused workflow. |
 
 ### `build_grouping_justification` parameters
 
@@ -526,12 +529,53 @@ Portable cross-suite contract files are versioned separately under `schemas/`.
 
 ---
 
+## Human review checkpoints
+
+For scientific governance, `run_oqt_multiagent_workflow` supports an optional `require_human_review=true` mode that pauses the workflow at high-risk decision points instead of auto-generating artifacts.
+
+### Checkpoints created
+1. **`chemical_identity`** — After resolving the input identifier to a Toolbox record.
+2. **`ad_assessment`** — When any QSAR prediction reports `ad_warning=true` (out of applicability domain).
+3. **`final_report`** — Before generating the PDF artifact.
+
+### Workflow behavior
+- If no checkpoints are triggered, the workflow completes normally (`status: "ok"`).
+- If checkpoints are pending, the workflow returns:
+  - `status: "review_required"`
+  - `workflow_id` — ID to use when resuming
+  - `review_checkpoints` — List of pending checkpoints with metadata
+  - **No PDF is generated.**
+
+### Approving or rejecting checkpoints
+Use the `approve_workflow_checkpoint` tool:
+
+```json
+{
+  "name": "approve_workflow_checkpoint",
+  "arguments": {
+    "checkpoint_id": "<checkpoint-id>",
+    "decision": "approved",
+    "comments": "Looks correct"
+  }
+}
+```
+
+Decisions: `approved` | `rejected`
+
+### Resuming the workflow
+Pass the same `workflow_id` back to `run_oqt_multiagent_workflow` (with `require_human_review=true`). The server will detect approved checkpoints and complete the workflow, returning `status: "ok"` and the PDF.
+
+> **Note:** Checkpoint state is held in memory. If the server restarts, pending checkpoints are lost. Do not use this feature for long-lived review cycles without external persistence.
+
+---
+
 ## Security checklist
 
 - ✅ Use OAuth2/OIDC in production (`BYPASS_AUTH=false`).
 - ✅ Terminate TLS at a reverse proxy.
 - ✅ Configure RBAC in `config/tool_permissions.default.json`.
 - ✅ Enable audit log shipping (see [docs/observability.md](docs/observability.md)).
+- ✅ Turn on `require_human_review=true` for high-stakes workflows to enforce explicit checkpoint approval.
 - ✅ Rotate secrets via platform-specific secret stores.
 - ✅ Regularly update the Docker base image (see [Dockerfile](Dockerfile)).
 

config/tool_permissions.default.json

@@ -13,6 +13,7 @@
     "list_qsar_models",
     "list_all_qsar_models",
     "build_grouping_justification",
+    "approve_workflow_checkpoint",
     "run_oqt_multiagent_workflow",
     "run_qsar_workflow",
     "canonicalize_structure",
@@ -38,6 +39,7 @@
     "list_qsar_models",
     "list_all_qsar_models",
     "build_grouping_justification",
+    "approve_workflow_checkpoint",
     "run_oqt_multiagent_workflow",
     "run_qsar_workflow",
     "run_qsar_model",
@@ -72,6 +74,7 @@
     "list_qsar_models",
     "list_all_qsar_models",
     "build_grouping_justification",
+    "approve_workflow_checkpoint",
     "run_oqt_multiagent_workflow",
     "run_qsar_workflow",
     "run_qsar_model",
@@ -106,6 +109,7 @@
     "list_qsar_models",
     "list_all_qsar_models",
     "build_grouping_justification",
+    "approve_workflow_checkpoint",
     "run_oqt_multiagent_workflow",
     "run_qsar_workflow",
     "run_qsar_model",

src/api/server.py

@@ -98,16 +98,29 @@ async def add_security_headers(request: Request, call_next):
 # Placeholder: In production, this must log to a centralized, tamper-evident system.
 @app.middleware("http")
 async def audit_log_middleware(request: Request, call_next):
+    from urllib.parse import parse_qs
+    from src.utils.privacy import scrub_dict, scrub_value
+
     correlation_id = str(uuid.uuid4())
     request.state.correlation_id = correlation_id
     start = time.perf_counter()
 
+    # Scrub potentially sensitive path segments (e.g., SMILES in URL paths)
+    scrubbed_path = scrub_value("path", request.url.path)
+    query_str = str(request.url.query) if request.url.query else ""
+    # Parse query string into a dict so keys remain readable and only values are scrubbed
+    if query_str:
+        parsed_query = parse_qs(query_str, keep_blank_values=True)
+        scrubbed_query = scrub_dict(parsed_query)
+    else:
+        scrubbed_query = {}
+
     log.debug(
         "Incoming request",
         extra={
             "cid": correlation_id,
             "method": request.method,
-            "path": request.url.path,
+            "path": scrubbed_path,
         },
     )
 
@@ -120,10 +133,12 @@ async def audit_log_middleware(request: Request, call_next):
         "correlation_id": correlation_id,
         "user_id": user_id,
         "method": request.method,
-        "path": request.url.path,
+        "path": scrubbed_path,
         "status_code": response.status_code,
         "duration_ms": round(duration_ms, 3),
     }
+    if scrubbed_query:
+        event["query"] = scrubbed_query
     audit.emit(event)
 
     response.headers["X-Request-ID"] = correlation_id

src/integrations/oqt_assistant.py

@@ -23,6 +23,8 @@
 from dataclasses import dataclass
 from typing import Any, Dict, List, Optional, Tuple
 
+from src.utils.sanitization import sanitize_for_llm
+
 logger = logging.getLogger(__name__)
 
 try:
@@ -313,21 +315,26 @@ async def _run_agents(
     """
     Execute the oqt_assistant specialist agents and synthesiser.
     """
+    # Harden LLM-facing inputs against prompt-boundary confusion (OQT-04 / SEC-02)
+    safe_identifier = sanitize_for_llm(identifier)
+    safe_context = sanitize_for_llm(context)
+
     specialist_outputs: Dict[str, str] = {}
     logger.info("  [Analysis] Starting assistant agents.")
 
     try:
         identity_txt = await analyze_chemical_context(
             {"basic_info": bundle.get("chemical_data", {}).get("basic_info", {})},
-            context,
+            safe_context,
             llm_config,
         )
     except Exception as exc:  # pragma: no cover - defensive branch
         logger.error("Chemical context agent failed: %s", exc)
         identity_txt = f"[Chemical Context agent failed: {exc}]"
 
-    specialist_outputs["Chemical_Context"] = identity_txt
-    analysis_context = f"{identity_txt}\n\nUser Goal: {context}"
+    safe_identity_txt = sanitize_for_llm(identity_txt)
+    specialist_outputs["Chemical_Context"] = safe_identity_txt
+    analysis_context = f"{safe_identity_txt}\n\nUser Goal: {safe_context}"
 
     props = bundle.get("chemical_data", {}).get("properties", {})
     profiling = bundle.get("profiling", {})
@@ -389,11 +396,12 @@ async def _run_agents(
         logger.error("Read-Across agent failed: %s", exc)
         read_across = f"[Read Across agent failed: {exc}]"
 
-    specialist_outputs["Read_Across"] = read_across
+    safe_read_across = sanitize_for_llm(read_across)
+    specialist_outputs["Read_Across"] = safe_read_across
 
     try:
         final_report = await synthesize_report(
-            identifier, core_outputs, read_across, context, llm_config
+            safe_identifier, core_outputs, safe_read_across, safe_context, llm_config
         )
     except Exception as exc:  # pragma: no cover - defensive branch
         logger.error("Report synthesis failed: %s", exc)

src/qsar/client.py

@@ -142,6 +142,8 @@ async def _execute_request() -> Tuple[Any, Dict[str, Any]]:
                             "last_attempt_ms": round(elapsed_attempt, 3),
                             "timeout_profile": profile,
                             "status_code": response.status_code,
+                            "api_versions": response.headers.get("api-supported-versions"),
+                            "server_date": response.headers.get("date"),
                         }
                         return data, meta
 

src/tools/implementations/o_qt_qsar_tools.py

@@ -63,7 +63,7 @@ class ChemicalSearchParams(BaseModel):
         ..., description="The search term (Name, CAS number, or SMILES)."
     )
     search_type: str = Field(
-        "auto", description="Type of search (e.g., 'auto', 'name', 'cas', 'smiles')."
+        "name", description="Type of search (e.g., 'auto', 'name', 'cas', 'smiles')."
     )
 
 
@@ -375,13 +375,28 @@ async def run_qsar_prediction(smiles: str, model_id: str) -> dict:
         )
         return _attach_toolbox(result, toolbox_meta)
 
+    # Light-weight applicability-domain gating (OQT-01)
+    domain_value = ""
+    if isinstance(domain, dict):
+        domain_value = domain.get("DomainResult") or domain.get("Domain") or ""
+    elif isinstance(domain, str):
+        domain_value = domain
+    domain_normalized = str(domain_value).strip().replace(" ", "").replace("-", "").lower()
+    ad_warning = domain_normalized in {"outofdomain", "out_of_domain"}
+
     result = {
         "chem_id": chem_id,
         "model_id": model_id,
         "prediction": prediction,
         "domain": domain,
-        "search_hits": hits,
+        "ad_status": "out_of_domain" if ad_warning else ("in_domain" if domain_normalized in {"indomain", "in_domain", "insideapplicabilitydomain"} else "unknown"),
+        "ad_warning": ad_warning,
     }
+    if ad_warning:
+        result["ad_recommendation"] = (
+            "This prediction is outside the model's applicability domain. "
+            "Treat with caution and consider experimental validation or read-across."
+        )
     if model_provenance:
         result["model_provenance"] = model_provenance
     toolbox_meta = _aggregate_meta(

src/tools/implementations/toolbox_execution.py

@@ -254,12 +254,34 @@ async def run_qsar_model(qsar_guid: str, chem_id: str) -> dict:
         _format_meta("qsar/domain", domain_meta),
         _format_meta("about/object", model_meta),
     )
+    # Light-weight applicability-domain gating (OQT-01)
+    domain_value = ""
+    if isinstance(domain, dict):
+        domain_value = domain.get("DomainResult") or domain.get("Domain") or ""
+    elif isinstance(domain, str):
+        domain_value = domain
+    domain_normalized = str(domain_value).strip().replace(" ", "").replace("-", "").lower()
+    ad_warning = domain_normalized in {"outofdomain", "out_of_domain"}
+
     result = {
         "qsar_guid": qsar_guid,
         "chem_id": chem_id,
         "prediction": prediction,
         "domain": domain,
+        "ad_status": "out_of_domain"
+        if ad_warning
+        else (
+            "in_domain"
+            if domain_normalized in {"indomain", "in_domain", "insideapplicabilitydomain"}
+            else "unknown"
+        ),
+        "ad_warning": ad_warning,
     }
+    if ad_warning:
+        result["ad_recommendation"] = (
+            "This prediction is outside the model's applicability domain. "
+            "Treat with caution and consider experimental validation or read-across."
+        )
     if model_provenance:
         result["model_provenance"] = model_provenance
     return _attach_toolbox(result, toolbox_meta)