Skip to content

fix(langfuse): require explicit export consent - #64236

Open
mudrii wants to merge 1 commit into
NousResearch:mainfrom
mudrii:fix/langfuse-export-consent
Open

fix(langfuse): require explicit export consent#64236
mudrii wants to merge 1 commit into
NousResearch:mainfrom
mudrii:fix/langfuse-export-consent

Conversation

@mudrii

@mudrii mudrii commented Jul 14, 2026

Copy link
Copy Markdown

Summary

  • register no Langfuse export hooks until explicit config consent is present
  • distinguish metadata export from full content export
  • omit prompts, completions, tool arguments, and tool results in metadata-only mode
  • document the exact exported fields and fail-closed default

Privacy rationale

Enabling a plugin and configuring credentials is not consent to export conversation content. observability.langfuse.export now defaults effectively to none; operators may choose metadata or content explicitly. Metadata mode retains operational trace value without transmitting prompts, completions, or tool payloads.

Verification

  • /Users/mudrii/.hermes/hermes-agent/venv/bin/python -m pytest -q tests/plugins/test_langfuse_plugin.py
  • 59 passed, 0 failed
  • ruff check passed
  • git diff --check passed

Copilot AI review requested due to automatic review settings July 14, 2026 06:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the bundled observability/langfuse plugin so it fails closed by default and only exports traces after explicit, non-env-based consent is present in config.yaml. It also introduces a two-tier export model (metadata vs content) intended to preserve operational observability while preventing prompt/tool payload exfiltration unless the operator opts in.

Changes:

  • Add explicit consent parsing/loading (none/metadata/content) and gate both hook registration and client initialization on consent.
  • Implement metadata-only redaction across LLM/tool hooks (omit prompt/completion/tool args/results while still emitting counters/usage/cost).
  • Update plugin README and extend tests for consent coercion + “no-consent means no hooks/client” behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
plugins/observability/langfuse/__init__.py Adds explicit consent loading + gates export; redacts payloads in metadata mode.
plugins/observability/langfuse/README.md Documents consent requirement and enumerates exported fields per consent level.
tests/plugins/test_langfuse_plugin.py Adds consent coercion tests and verifies no hooks/client without consent.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +486 to +501
@pytest.mark.parametrize("value", [True, 1, "true", "1", "yes", "on"])
def test_consent_rejects_legacy_truthy_values(self, monkeypatch, value):
self._clear_env(monkeypatch)
plugin = self._fresh_plugin()
assert plugin._coerce_consent_level(value) == "none"

@pytest.mark.parametrize(
("value", "expected"),
[("metadata", "metadata"), ("content", "content")],
)
def test_consent_accepts_only_explicit_levels(
self, monkeypatch, value, expected
):
self._clear_env(monkeypatch)
plugin = self._fresh_plugin()
assert plugin._coerce_consent_level(value) == expected
@alt-glitch alt-glitch added type/feature New feature or request comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have labels Jul 14, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for hardening the existing Langfuse export path. The privacy premise is valid: current main serializes request messages, assistant output, tool arguments, and tool results in plugins/observability/langfuse/__init__.py:891-905, :937-951, and :1060-1125.

Problems

  • hermes_cli/tools_config.py:555-576 and :1395-1423 collect credentials and enable the plugin, but never ask for or save the new consent value. With this patch, that supported setup path enables a plugin that exports nothing.
  • The changed plugin README is not sufficient: website/docs/user-guide/features/built-in-plugins.md:149-204 still says setup makes the next turn ship a trace and documents content-bearing exports.
  • The new tests do not exercise a real temp config.yaml or verify metadata/content payload boundaries.

Suggested changes

  • Add an explicit consent choice to the existing Langfuse setup flow and persist one documented config.yaml key.
  • Update the shipped Langfuse guides and add config-to-observation tests for no-consent, metadata, and content modes.

Automated hermes-sweeper review.

open.
Without the SDK, credentials, or explicit export consent, the plugin does not
register export hooks and does not initialize a Langfuse client.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also wire this documented setting into the existing hermes tools Langfuse setup flow. That flow currently only collects credentials and enables the plugin (hermes_cli/tools_config.py:555-576, :1395-1423); without a consent prompt/config write, every wizard-configured installation will be fail-closed indefinitely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants