fix(security): require explicit consent before uploading debug logs

`hermes debug share` printed a privacy notice and then uploaded the report to a public paste service in the same breath — the user never got to say yes or no. Add a consent gate: an interactive [y/N] prompt, a --yes/-y flag to skip it, and a hard refusal (exit 1) in non-interactive contexts (no TTY on stdin) so debug data can't be exposed silently in scripts/CI. - New _confirm_upload() helper gates the actual upload after the notice. - Applied to BOTH upload paths: the public paste.rs path and the --nous Nous-S3 path (the latter is a sibling site the original PR missed). - The /debug slash command passes yes=True (typing /debug is itself the consent action, and input() would hang inside prompt_toolkit). - Rewrote the privacy notice for accuracy: secrets (API keys/tokens/ passwords) ARE force-redacted before upload; PII (display name, platform user ID, verbatim message content, filesystem paths) is NOT, and that URL is public. Fixes #22016. Co-authored-by: liuhao1024 <liuhao1024@users.noreply.github.com>
2026-07-01 00:17:16 -07:00
parent 3aebdb1d23
commit 54f32af4a7
4 changed files with 204 additions and 14 deletions
@@ -2593,7 +2593,12 @@ class CLICommandsMixin:
        words = {w.lower() for w in cmd_original.split()[1:]}
        local = "local" in words
        nous = "nous" in words and not local
-        args = SimpleNamespace(lines=200, expire=7, local=local, nous=nous)
+        # Typing the /debug slash command is itself the explicit consent to
+        # upload, so we pass yes=True to skip run_debug_share's [y/N] prompt.
+        # input() would hang inside prompt_toolkit's event loop anyway.
+        args = SimpleNamespace(
+            lines=200, expire=7, local=local, nous=nous, yes=True
+        )
        run_debug_share(args)

    def _handle_update_command(self) -> bool:
@@ -196,15 +196,19 @@ def _best_effort_sweep_expired_pastes() -> None:
 # ---------------------------------------------------------------------------

 _PRIVACY_NOTICE = """\
-⚠️  This will upload the following to a public paste service:
-  • System info (OS, Python version, Hermes version, provider, which API keys
-    are configured — NOT the actual keys)
-  • Recent log lines (agent.log, errors.log, gateway.log, gui.log, desktop.log
-    — may contain conversation fragments and file paths)
-  • Full agent.log, gateway.log, gui.log, and desktop.log (up to 512 KB each —
-    likely contains conversation content, tool outputs, and file paths)
+⚠️  This will upload system info + logs to a PUBLIC paste service.

-Pastes auto-delete after 6 hours.
+Cryptographic secrets (API keys, tokens, passwords) are redacted before
+upload, but the following personal data is NOT redacted and will be public:
+  • Your display name and persistent platform user ID
+  • Verbatim content of your recent messages (prompts, responses, tool output)
+  • Local filesystem paths
+  • Any other PII present in the logs
+
+The resulting URL is public to anyone who has the link. Pastes auto-delete
+after 6 hours, but may be archived by third parties in the meantime.
+
+Use --local to view the report without uploading.
 """

 _GATEWAY_PRIVACY_NOTICE = (
@@ -774,6 +778,38 @@ def build_debug_share(
    )


+def _confirm_upload(args) -> bool:
+    """Require explicit consent before any debug-share upload.
+
+    The privacy notice is printed by the caller. This gates the actual
+    upload: with ``--yes`` (or ``-y``) we proceed unprompted; otherwise we
+    ask an interactive ``[y/N]`` question. In a non-interactive context
+    (no TTY on stdin — scripts, CI, piped input) we refuse rather than
+    hang or upload silently, so debug data can't be exposed without a
+    deliberate ``--yes``.
+
+    Returns True to proceed with the upload, False to abort.
+    """
+    if bool(getattr(args, "yes", False)):
+        return True
+    if not sys.stdin.isatty():
+        print(
+            "ERROR: Non-interactive mode requires --yes to confirm upload.\n"
+            "       This prevents accidental exposure of personal data.\n"
+            "       Use --local to view the report without uploading.",
+            file=sys.stderr,
+        )
+        sys.exit(1)
+    try:
+        answer = input("Upload debug report? [y/N] ").strip().lower()
+    except (EOFError, KeyboardInterrupt):
+        answer = ""
+    if answer not in ("y", "yes"):
+        print("Aborted.")
+        return False
+    return True
+
+
 def run_debug_share(args):
    """Collect debug report + full logs, upload each, print URLs."""
    log_lines = getattr(args, "lines", 200)
@@ -805,10 +841,12 @@ def run_debug_share(args):
        return

    if nous:
-        _run_debug_share_nous(log_lines=log_lines, redact=redact)
+        _run_debug_share_nous(args, log_lines=log_lines, redact=redact)
        return

    print(_PRIVACY_NOTICE)
+    if not _confirm_upload(args):
+        return
    print("Collecting debug report...")
    print("Uploading...")

@@ -856,7 +894,7 @@ _NOUS_PRIVACY_NOTICE = """\
 """


-def _run_debug_share_nous(*, log_lines: int, redact: bool) -> None:
+def _run_debug_share_nous(args, *, log_lines: int, redact: bool) -> None:
    """Handle ``hermes debug share --nous``: upload the bundle to Nous-S3.

    Collects the same force-redacted bundle as the paste path, gzips it into
@@ -867,6 +905,8 @@ def _run_debug_share_nous(*, log_lines: int, redact: bool) -> None:
    from hermes_cli.diagnostics_upload import share_to_nous

    print(_NOUS_PRIVACY_NOTICE)
+    if not _confirm_upload(args):
+        return
    if not redact:
        print(
            "⚠️  --no-redact is set: secrets in your logs will NOT be redacted "
@@ -24,7 +24,8 @@ def build_debug_parser(subparsers, *, cmd_debug: Callable) -> None:
        formatter_class=argparse.RawDescriptionHelpFormatter,
        epilog="""\
 Examples:
-    hermes debug share              Upload debug report and print URL
+    hermes debug share              Upload debug report (asks for confirmation)
+    hermes debug share --yes        Skip confirmation (for scripts/CI)
    hermes debug share --lines 500  Include more log lines
    hermes debug share --expire 30  Keep paste for 30 days
    hermes debug share --local      Print report locally (no upload)
@@ -55,6 +56,16 @@ Examples:
        action="store_true",
        help="Print the report locally instead of uploading",
    )
+    share_parser.add_argument(
+        "-y",
+        "--yes",
+        action="store_true",
+        help=(
+            "Skip the confirmation prompt and upload immediately. Required "
+            "in non-interactive contexts (scripts/CI); without it, and with "
+            "no TTY on stdin, the command refuses rather than upload silently."
+        ),
+    )
    share_parser.add_argument(
        "--no-redact",
        action="store_true",
@@ -1289,7 +1289,8 @@ class TestShareIncludesAutoDelete:
            run_debug_share(args)

        out = capsys.readouterr().out
-        assert "public paste service" in out
+        assert "PUBLIC paste service" in out
+        assert "NOT redacted" in out

    def test_local_no_privacy_notice(self, hermes_home, capsys):
        from hermes_cli.debug import run_debug_share
@@ -1304,7 +1305,7 @@ class TestShareIncludesAutoDelete:
            run_debug_share(args)

        out = capsys.readouterr().out
-        assert "public paste service" not in out
+        assert "PUBLIC paste service" not in out


 # ---------------------------------------------------------------------------
@@ -1519,6 +1520,7 @@ class TestRunDebugShareNous:
            local = False
            nous = True
            no_redact = False
+            yes = True

        a = _A()
        for k, v in over.items():
@@ -1602,6 +1604,9 @@ class TestDebugSlashCommand:
        c = self._captured("/debug")
        assert c["nous"] is False and c["local"] is False
        assert c["lines"] == 200 and c["expire"] == 7
+        # The slash command IS the consent action → skip the [y/N] prompt
+        # (input() would hang inside prompt_toolkit's event loop).
+        assert c["yes"] is True

    def test_nous_word_sets_nous(self):
        c = self._captured("/debug nous")
@@ -1629,3 +1634,132 @@ class TestDebugSlashCommand:
        c = self._captured("")
        assert c["nous"] is False and c["local"] is False

+
+class TestShareConsentGate:
+    """`hermes debug share` requires explicit consent before uploading.
+
+    Uses SimpleNamespace rather than MagicMock so ``args.yes`` is a real
+    ``False`` — a MagicMock auto-provides a truthy ``.yes`` and would silently
+    bypass the very gate under test.
+    """
+
+    def _args(self, **over):
+        from types import SimpleNamespace
+
+        base = dict(lines=50, expire=7, local=False, nous=False,
+                    no_redact=False, yes=False)
+        base.update(over)
+        return SimpleNamespace(**base)
+
+    def test_aborts_on_user_decline(self, hermes_home, capsys, monkeypatch):
+        """Interactive user typing anything but y/yes → no upload."""
+        from hermes_cli.debug import run_debug_share
+
+        monkeypatch.setattr("sys.stdin.isatty", lambda: True)
+        monkeypatch.setattr("builtins.input", lambda _: "n")
+
+        with patch("hermes_cli.dump.run_dump"), \
+             patch("hermes_cli.debug.upload_to_pastebin") as mock_upload:
+            run_debug_share(self._args())
+
+        mock_upload.assert_not_called()
+        assert "Aborted" in capsys.readouterr().out
+
+    def test_proceeds_on_user_accept(self, hermes_home, capsys, monkeypatch):
+        """Interactive user typing 'y' → upload proceeds."""
+        from hermes_cli.debug import run_debug_share
+
+        monkeypatch.setattr("sys.stdin.isatty", lambda: True)
+        monkeypatch.setattr("builtins.input", lambda _: "y")
+
+        with patch("hermes_cli.dump.run_dump"), \
+             patch("hermes_cli.debug._sweep_expired_pastes", return_value=(0, 0)), \
+             patch("hermes_cli.debug.upload_to_pastebin",
+                   return_value="https://paste.rs/test"), \
+             patch("hermes_cli.debug._schedule_auto_delete"):
+            run_debug_share(self._args())
+
+        out = capsys.readouterr().out
+        assert "Debug report uploaded" in out
+        assert "Aborted" not in out
+
+    def test_yes_flag_skips_prompt(self, hermes_home, capsys, monkeypatch):
+        """--yes uploads without ever calling input()."""
+        from hermes_cli.debug import run_debug_share
+
+        def _boom(_):
+            raise AssertionError("input() must not be called with --yes")
+
+        monkeypatch.setattr("builtins.input", _boom)
+
+        with patch("hermes_cli.dump.run_dump"), \
+             patch("hermes_cli.debug._sweep_expired_pastes", return_value=(0, 0)), \
+             patch("hermes_cli.debug.upload_to_pastebin",
+                   return_value="https://paste.rs/test"), \
+             patch("hermes_cli.debug._schedule_auto_delete"):
+            run_debug_share(self._args(yes=True))
+
+        assert "Debug report uploaded" in capsys.readouterr().out
+
+    def test_non_interactive_requires_yes(self, hermes_home, capsys, monkeypatch):
+        """No TTY + no --yes → exit(1), never upload silently."""
+        from hermes_cli.debug import run_debug_share
+
+        monkeypatch.setattr("sys.stdin.isatty", lambda: False)
+
+        with patch("hermes_cli.dump.run_dump"), \
+             patch("hermes_cli.debug.upload_to_pastebin") as mock_upload:
+            with pytest.raises(SystemExit) as exc:
+                run_debug_share(self._args())
+
+        assert exc.value.code == 1
+        mock_upload.assert_not_called()
+        err = capsys.readouterr().err
+        assert "Non-interactive mode requires --yes" in err
+        assert "personal data" in err
+
+    def test_non_interactive_with_yes_succeeds(self, hermes_home, capsys, monkeypatch):
+        """No TTY but --yes present → upload proceeds."""
+        from hermes_cli.debug import run_debug_share
+
+        monkeypatch.setattr("sys.stdin.isatty", lambda: False)
+
+        with patch("hermes_cli.dump.run_dump"), \
+             patch("hermes_cli.debug._sweep_expired_pastes", return_value=(0, 0)), \
+             patch("hermes_cli.debug.upload_to_pastebin",
+                   return_value="https://paste.rs/test"), \
+             patch("hermes_cli.debug._schedule_auto_delete"):
+            run_debug_share(self._args(yes=True))
+
+        assert "https://paste.rs/test" in capsys.readouterr().out
+
+    def test_nous_path_also_gated(self, hermes_home, capsys, monkeypatch):
+        """The --nous S3 path enforces the same consent gate (sibling site)."""
+        from hermes_cli.debug import run_debug_share
+
+        monkeypatch.setattr("sys.stdin.isatty", lambda: True)
+        monkeypatch.setattr("builtins.input", lambda _: "n")
+
+        with patch("hermes_cli.dump.run_dump"), \
+             patch("hermes_cli.diagnostics_upload.share_to_nous") as mock_nous:
+            run_debug_share(self._args(nous=True))
+
+        mock_nous.assert_not_called()
+        assert "Aborted" in capsys.readouterr().out
+
+    def test_local_never_prompts(self, hermes_home, capsys, monkeypatch):
+        """--local renders to stdout and must not prompt or upload."""
+        from hermes_cli.debug import run_debug_share
+
+        def _boom(_):
+            raise AssertionError("input() must not be called for --local")
+
+        monkeypatch.setattr("builtins.input", _boom)
+
+        with patch("hermes_cli.dump.run_dump"), \
+             patch("hermes_cli.debug.upload_to_pastebin") as mock_upload:
+            run_debug_share(self._args(local=True))
+
+        mock_upload.assert_not_called()
+        assert "Aborted" not in capsys.readouterr().out
+