fix(share): always-on redaction boundary + rebase/UX fixes (gate findings)

Adversarial security re-gate on the rebased head found 4 issues; fixed: 1. [CORE] api/shares.py public snapshot could leak credentials/paths/tool payloads. The sanitizer relied on redact_session_data (gated on the user-toggleable api_redact_enabled) — disabling it exposed API keys, a dict assistant content was stringified as raw tool data, and session.workspace survived in transcript text. Now the per-message sanitizer applies ALWAYS-ON credential redaction (_redact_fn_cached, force=True, ignores the setting) + scrubs known session/workspace/home paths, and refuses to stringify non-text structured content. Title is force-redacted too. Regression test proves the boundary holds with api_redact_enabled=False. 2. [SILENT] api/models.py: my rebase wrongly kept anchor_activity_scenes in METADATA_FIELDS, moving 250-480KB scene bodies before messages and undoing master's metadata fast-path (#5854). Removed; 17 #5854 tests pass again. 3. [SILENT] static/index.html: the Share/StopSharing/ExportHTML button triplet was double-applied by the rebase (duplicate IDs → only the first wired). Deduped to one each + a count assertion test. 4. tests: fixed the stale Session.load-after-/api/session/new setup (master keeps new sessions memory-only until first message). Also (UX, found in the visual flow sweep): a clipboard-copy failure after a SUCCESSFUL share showed a red 'Share failed' toast and skipped opening the page. The copy is now isolated — the share succeeds, the page opens, and the toast shows the link if the clipboard write fails, never a false failure. Co-authored-by: MicoRobot.
2026-07-11 02:06:34 +00:00
1 parent e061e8534c
commit 75153f5b79
@@ -1270,7 +1270,7 @@ class Session:
            'parent_session_id',
            'worktree_path', 'worktree_branch', 'worktree_repo_root', 'worktree_created_at',
            'is_cli_session', 'source_tag', 'raw_source', 'session_source', 'source_label', 'read_only',
-            'enabled_toolsets', 'composer_draft', 'anchor_activity_scenes',
+            'enabled_toolsets', 'composer_draft',
            'process_wakeup_pause',
            'share_token', 'share_created_at',
        ]
@@ -19,6 +19,12 @@ from pathlib import Path

 from api.config import STATE_DIR
 from api.helpers import redact_session_data
+# _redact_fn_cached is the ALWAYS-ON credential redactor (agent redactor with
+# force=True + local fallback regex). Unlike redact_session_data it does NOT
+# consult the user-toggleable api_redact_enabled setting — a public share is a
+# hard safety boundary that must redact credentials even if the operator turned
+# API-response redaction off.
+from api.helpers import _redact_fn_cached as _force_redact_credentials

 logger = logging.getLogger(__name__)

@@ -67,14 +73,38 @@ def _share_message_text(message: dict) -> str:
        parts = []
        for item in content:
            if not isinstance(item, dict):
+                # Non-dict list items (e.g. nested structures) are NOT plain text —
+                # never stringify them into the public snapshot.
                continue
            if item.get("type") == "text":
                parts.append(str(item.get("text") or ""))
        return "".join(parts).strip()
-    return str(content or "").strip()
+    if isinstance(content, str):
+        return content.strip()
+    # A dict/other structured content (e.g. a tool-result object) is NOT shareable
+    # text — do NOT str() it (that would publish raw structured/tool payload).
+    return ""


-def _sanitize_message(message: dict) -> dict | None:
+def _redact_share_paths(text: str, extra_paths) -> str:
+    """Strip known local session/workspace/home paths out of public-share text.
+
+    A workspace path or Hermes home can be embedded inside message prose (an
+    agent quoting a file path, a traceback, etc.). Redact the concrete local
+    paths so a public share never discloses the operator's filesystem layout.
+    """
+    if not isinstance(text, str) or not text:
+        return text
+    for p in extra_paths:
+        if not p:
+            continue
+        p = str(p).strip()
+        if len(p) >= 4 and p in text:
+            text = text.replace(p, "[redacted-path]")
+    return text
+
+
+def _sanitize_message(message: dict, *, redact_paths=()) -> dict | None:
    if not isinstance(message, dict):
        return None
    role = str(message.get("role") or "").strip().lower()
@@ -83,6 +113,12 @@ def _sanitize_message(message: dict) -> dict | None:
    text = _share_message_text(message)
    if not text:
        return None
+    # ALWAYS-ON hardening for the public boundary, independent of any setting:
+    # (1) force credential redaction, (2) strip known local paths.
+    text = _force_redact_credentials(text)
+    text = _redact_share_paths(text, redact_paths)
+    if not text.strip():
+        return None
    sanitized = {
        "role": role,
        "content": text,
@@ -112,16 +148,38 @@ def _public_share_payload(payload: dict) -> dict:


 def build_share_snapshot(session) -> dict:
-    safe_session = redact_session_data(getattr(session, "__dict__", {}) or {})
+    raw_dict = getattr(session, "__dict__", {}) or {}
+    # redact_session_data respects the api_redact_enabled setting; keep it as a
+    # first pass, but the per-message sanitizer below applies ALWAYS-ON credential
+    # + path redaction that does NOT depend on that setting (the public boundary
+    # must hold even if the operator disabled api_redact_enabled).
+    safe_session = redact_session_data(raw_dict)
+    # Concrete local paths to scrub from any message prose / title.
+    redact_paths = []
+    for key in ("workspace", "worktree_path", "worktree_repo_root"):
+        val = raw_dict.get(key)
+        if val:
+            redact_paths.append(str(val))
+    try:
+        from api.profiles import get_active_hermes_home
+        redact_paths.append(str(get_active_hermes_home()))
+    except Exception:
+        pass
+    try:
+        redact_paths.append(str(Path.home()))
+    except Exception:
+        pass
    safe_messages = []
    for raw in safe_session.get("messages") or []:
-        sanitized = _sanitize_message(raw)
+        sanitized = _sanitize_message(raw, redact_paths=redact_paths)
        if sanitized:
            safe_messages.append(sanitized)
    if not safe_messages:
        raise ValueError("This conversation has no shareable messages yet.")
+    title = _force_redact_credentials(str(safe_session.get("title") or "Untitled"))
+    title = _redact_share_paths(title, redact_paths) or "Untitled"
    return {
-        "title": str(safe_session.get("title") or "Untitled"),
+        "title": title,
        "messages": safe_messages,
        "message_count": len(safe_messages),
    }
@@ -943,9 +943,6 @@
              <button class="settings-action-btn" id="btnShareSession" title="Create a public read-only share link" data-i18n-title="share_session_tooltip"><svg width="13" height="13" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><circle cx="18" cy="5" r="3"/><circle cx="6" cy="12" r="3"/><circle cx="18" cy="19" r="3"/><path d="M8.6 13.5l6.8 4"/><path d="M15.4 6.5 8.6 10.5"/></svg> <span data-i18n="share_session">Share</span></button>
              <button class="settings-action-btn danger" id="btnStopSharingSession" title="Revoke the public read-only share link for this conversation" data-i18n-title="stop_sharing_session_tooltip"><svg width="13" height="13" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><path d="M18 8a6 6 0 0 0-8.49 0L6 11.5"/><path d="M18 16a6 6 0 0 1-8.49 0L6 12.5"/><line x1="3" y1="3" x2="21" y2="21"/></svg> <span data-i18n="stop_sharing_session">Stop sharing</span></button>
              <button class="settings-action-btn" id="btnExportHTML" title="Export as a self-contained HTML page" data-i18n-title="export_session_html_tooltip"><svg width="13" height="13" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><path d="M14 2H6a2 2 0 0 0-2 2v16a2 2 0 0 0 2 2h12a2 2 0 0 0 2-2V8z"/><polyline points="14 2 14 8 20 8"/><path d="M9 13l-2 2 2 2"/><path d="M15 13l2 2-2 2"/></svg> <span data-i18n="export_session_html">HTML</span></button>
-              <button class="settings-action-btn" id="btnShareSession" title="Create a public read-only share link" data-i18n-title="share_session_tooltip"><svg width="13" height="13" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><circle cx="18" cy="5" r="3"/><circle cx="6" cy="12" r="3"/><circle cx="18" cy="19" r="3"/><path d="M8.6 13.5l6.8 4"/><path d="M15.4 6.5 8.6 10.5"/></svg> <span data-i18n="share_session">Share</span></button>
-              <button class="settings-action-btn danger" id="btnStopSharingSession" title="Revoke the public read-only share link for this conversation" data-i18n-title="stop_sharing_session_tooltip"><svg width="13" height="13" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><path d="M18 8a6 6 0 0 0-8.49 0L6 11.5"/><path d="M18 16a6 6 0 0 1-8.49 0L6 12.5"/><line x1="3" y1="3" x2="21" y2="21"/></svg> <span data-i18n="stop_sharing_session">Stop sharing</span></button>
-              <button class="settings-action-btn" id="btnExportHTML" title="Export as a self-contained HTML page" data-i18n-title="export_session_html_tooltip"><svg width="13" height="13" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><path d="M14 2H6a2 2 0 0 0-2 2v16a2 2 0 0 0 2 2h12a2 2 0 0 0 2-2V8z"/><polyline points="14 2 14 8 20 8"/><path d="M9 13l-2 2 2 2"/><path d="M15 13l2 2-2 2"/></svg> <span data-i18n="export_session_html">HTML</span></button>
              <button class="settings-action-btn" id="btnImportJSON" title="Import session from JSON" data-i18n-title="import_session_json_tooltip"><svg width="13" height="13" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><path d="M21 15v4a2 2 0 0 1-2 2H5a2 2 0 0 1-2-2v-4"/><polyline points="17 8 12 3 7 8"/><line x1="12" y1="3" x2="12" y2="15"/></svg> <span data-i18n="import">Import</span></button>
              <button class="settings-action-btn danger" id="btnClearConvModal" onclick="clearConversation()" title="Clear all messages in this conversation" data-i18n-title="clear_conversation_btn_tooltip"><svg width="13" height="13" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><path d="M3 6h18"/><path d="M19 6v14c0 1-1 2-2 2H7c-1 0-2-1-2-2V6"/><path d="M8 6V4c0-1 1-2 2-2h4c1 0 1 2 2 2v2"/></svg> <span data-i18n="clear">Clear</span></button>
            </div>
@@ -4144,8 +4144,9 @@ async function _createOrRefreshSessionShare(session){
      cancelLabel:t('share_session_refresh_snapshot'),
    });
    if(reuse){
-      await _copyTextToClipboard(existing);
-      showToast(t('share_session_link_copied'));
+      let copied=true;
+      try{ await _copyTextToClipboard(existing); }catch(_){ copied=false; }
+      showToast(copied?t('share_session_link_copied'):(t('share_session_status_active')+' — '+existing),copied?2500:6000);
      window.open(existing,'_blank','noopener');
      return;
    }
@@ -4153,8 +4154,16 @@ async function _createOrRefreshSessionShare(session){
  const res=await api('/api/share/create',{method:'POST',body:JSON.stringify({session_id:session.session_id})});
  if(res&&res.session) _syncSessionShareState(session,res.session);
  const href=new URL(String(res&&res.share&&res.share.url||''),location.origin).href;
-  await _copyTextToClipboard(href);
-  showToast(existing?t('share_session_link_copied'):t('share_session_created'));
+  // The share is now created server-side. A clipboard-copy failure (permissions,
+  // focus, non-secure context) must NOT be reported as "Share failed" — the link
+  // exists and we still open it. Only surface the copied-vs-not-copied distinction.
+  let copied=true;
+  try{ await _copyTextToClipboard(href); }catch(_){ copied=false; }
+  if(copied){
+    showToast(existing?t('share_session_link_copied'):t('share_session_created'));
+  }else{
+    showToast((existing?t('share_session_created'):t('share_session_created'))+' — '+href,6000);
+  }
  window.open(href,'_blank','noopener');
 }

@@ -40,8 +40,10 @@ def _make_session_with_messages():
    sid = created["session"]["session_id"]
    from api.models import Session

-    session = Session.load(sid)
-    assert session is not None
+    # Current master keeps a freshly /api/session/new session memory-only until
+    # its first message is persisted, so Session.load(sid) can return None here.
+    # Construct + persist the session directly so the share path has a real file.
+    session = Session.load(sid) or Session(session_id=sid)
    session.title = "Shared Test"
    session.messages = [
        {"role": "system", "content": "internal system instructions should stay private"},
@@ -74,3 +74,54 @@ def test_session_action_menu_exposes_public_share_actions():
 def test_public_share_page_does_not_render_provider_details_blocks():
    assert "provider-error-details" not in SHARE_JS
    assert "provider_details_label||'Provider details'" not in SHARE_JS
+
+
+def test_share_action_buttons_are_not_duplicated():
+    # Regression: a rebase double-applied the Share/StopSharing/ExportHTML button
+    # triplet, leaving duplicate IDs so getElementById wired only the first copy
+    # (three visible-but-inert buttons). Each control must appear exactly once.
+    for _id in ("btnShareSession", "btnStopSharingSession", "btnExportHTML"):
+        assert INDEX_HTML.count(f'id="{_id}"') == 1, f"{_id} must appear exactly once"
+
+
+def test_share_snapshot_redaction_is_always_on_regardless_of_setting():
+    # The public-share boundary must redact credentials + local paths and drop
+    # non-text/tool/system content EVEN IF the operator disabled api_redact_enabled.
+    import os, tempfile, importlib
+    os.environ.setdefault("HERMES_WEBUI_STATE_DIR", tempfile.mkdtemp())
+    import api.config as config
+    import api.shares as shares
+    # Force the user-toggleable API redaction OFF (redact_session_data reads
+    # api.config.load_settings() at call time).
+    _orig = config.load_settings
+    config.load_settings = lambda: {"api_redact_enabled": False}
+    try:
+        class _S:
+            pass
+        s = _S()
+        s.title = "Deploy sk-ABCDEF1234567890TOKEN"
+        s.workspace = "/very/private/workspace"
+        s.messages = [
+            {"role": "system", "content": "internal system prompt stays private"},
+            {"role": "user", "content": "key sk-SECRET1234567890abcdef path /very/private/workspace/x.py"},
+            {"role": "assistant", "content": {"tool": "terminal", "output": "raw structured tool payload"}},
+            {"role": "tool", "content": "raw tool output"},
+            {"role": "assistant", "content": "Here is the answer."},
+        ]
+        snap = shares.build_share_snapshot(s)
+        import json
+        blob = json.dumps(snap)
+        # credentials masked (in both title and message text)
+        assert "sk-SECRET1234567890" not in blob
+        assert "sk-ABCDEF1234567890" not in blob
+        # local paths scrubbed
+        assert "/very/private/workspace" not in blob
+        # structured/dict content never stringified into the public payload
+        assert "raw structured tool payload" not in blob
+        # system + tool roles excluded
+        roles = [m["role"] for m in snap["messages"]]
+        assert "system" not in roles and "tool" not in roles
+        # the real answer is preserved
+        assert any("Here is the answer" in m["content"] for m in snap["messages"])
+    finally:
+        config.load_settings = _orig