Skip to content

Activate conformance fixture 158 payload-leak arms - #273

Merged
chris-colinsky merged 1 commit into
mainfrom
feature/0116-fixture-158-payload-leak
Aug 15, 2026
Merged

Activate conformance fixture 158 payload-leak arms#273
chris-colinsky merged 1 commit into
mainfrom
feature/0116-fixture-158-payload-leak

Conversation

@chris-colinsky

Copy link
Copy Markdown
Member

Activates spec conformance fixture 158 (158-langfuse-payload-leak-fail-closed) and flips proposal 0115 from partial to implemented. This completes the conformance side of the 0114-0118 provider-isolation arc: 157 and 158 now both run.

Case accounting

Against our langfuse_bound_provider_detection = true declaration, 158 asserts five of its eleven cases: the four raise arms plus the ungated opt-out. Five suppress-floor cases are recognized skips selected by requires_capability (§5.5), and one case is deferred by name because it drives a calls_rerank node this harness has no retrieval support for at all; it rides the retrieval cluster that 150/151 also wait on.

All six non-executing cases now surface as RecognizedSkip warnings on a passing run. Previously the per-case deferral was written into a local dict that is only rendered inside an assertion failure message, so a green run looked identical whether the fixture asserted eleven cases or five.

What the adversarial review changed

The first version of this passed, and three of its defects were real. Worth recording since two of them defeated the mutation testing that was supposed to catch exactly this:

The error-message channel was undetectable on the real client. The payload classifier read the bare langfuse.observation.metadata attribute, but a Langfuse v4 client only writes that name for non-dict metadata; a dict flattens to langfuse.observation.metadata.<key>. We always pass a dict, and every 158 case is mode: credentials, so a span carrying leaked exception text classified payload-free. The only test of that limb drove the fake, which was the one client writing a single blob, so the double both produced the evidence and judged it. The classifier now keys on the flattened attribute (parsing the bare blob for the non-dict case rather than substring-matching, which would false-positive on caller metadata values), the fake renders metadata the way a real client does, and a new test drives a real SDK client through the egress-free harness.

The log_records assertion matched the wrong record. Every 158 case primes the credential, so from_credentials always logs an unrelated cached-client warning on the same logger inside the capture window. Matching on level alone was satisfied by that record, and downgrading only the mandated emitter left the fixture green. It now discriminates on the emitting call site.

A case-level directive sat behind an early return. disable_provider_payload was read after the expected_construction_error branch returns, so it never executed for the only case that declares it while _CASE_DIRECTIVES vouched for it as implemented. The OTel observer is now built before the Langfuse construction and the runner asserts the flag reached it.

Also: the generic _run_case runner had been granted allowlist credit for six keys only the isolation runner reads, which would have converted a loud failure into silent coverage for any future fixture on that path. The allowlists are now per-runner. And the inspect.getsource guard on the trace-stub shapes was replaced with a behavioural one, since it could not detect a stub gaining a key, which is the exact drift its own comment claimed to catch.

Verification

2108 passed, 507 skipped. Ruff, pyright and the conformance-manifest check clean. Conformance and unit suites re-run together for order contamination, which matters here because the raise cases prime the SDK's real per-credential cache.

Each fix is mutation-verified, with the mutation confirmed to have landed before its result was trusted:

  • drop the flattened-key detection: red
  • downgrade only the mandated isolation WARNING: red
  • drop the case-level OTel flag wiring: red
  • add a key to the observer's trace stub: red

The one case that cannot be killed this way is singleton_preexists_raises_otel_not_suppressing, which is a negative control: construction raises before anything emits, so no conforming run can observe the flag. Spec has since confirmed the case is inert for every adapter under the current contract, because the case-level key is not in the adapter contract at all, and proposal 0120 will add a real otel_observer: directive to carry it.

Notes

Two findings from this work went to spec as items 11 and 12 of the consolidated v0.17.0 review, and both were ruled in our favour. Fixture 158's log_records cannot pin the WARNING it exists to pin, which is a gap in §6 rather than in the fixture, and case 2 discriminates nothing for anybody. Expect a follow-up rewiring this runner onto 0120's sanctioned spelling once that lands.

Fixture 158 exercises openarmature's fail-closed behaviour when a
Langfuse client is bound to a provider it does not own. Against our
detection-capable declaration it asserts five of its eleven cases: the
four raise arms plus the ungated opt-out. Five suppress-floor cases are
recognized skips selected by requires_capability, and one is deferred by
name because it drives a retrieval node this harness cannot yet build.
Both the skips and the deferral now surface on a passing run, so a case
that did not execute cannot read as coverage.

The provider-faithful double gains a payload-bearing classifier. It keys
on the flattened metadata attribute a real client writes rather than on
a single blob only the double emitted, which had made the harvested
error-message channel undetectable on the client mode every case uses.
The double now renders metadata the way a real client does, and a new
test drives a real SDK client so the double is no longer the only
witness for behaviour it exists to model.

The log_records assertion matches on the emitting call site rather than
on level alone. Construction always logs an unrelated cached-client
warning on the same logger, so a level-only match was satisfied by that
record while the mandated one went unchecked.

0115 moves to implemented now that both fixtures run.
Copilot AI lite review requested due to automatic review settings August 14, 2026 16:01
Comment thread tests/conformance/test_observability_langfuse.py

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This pull request activates spec conformance fixture 158 for Langfuse payload-leak “fail closed” behavior and updates the conformance harness and fakes so fixture 158’s assertions are observable and correctly classified against both the provider-faithful fake and the real Langfuse SDK path.

Changes:

  • Activates conformance fixture 158-langfuse-payload-leak-fail-closed alongside 157 and updates the harness to surface non-executing cases as RecognizedSkip warnings (capability-gated or per-case deferred).
  • Aligns the provider-faithful Langfuse fake with real v4 SDK metadata rendering (flattened langfuse.observation.metadata.<key> attributes) and adds payload-bearing classification utilities for leak assertions.
  • Updates unit and conformance tests to validate payload-bearing classification against both the fake and a real egress-free Langfuse SDK client, and strengthens log-record assertions by filtering on the emitting call site.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/unit/test_langfuse_provider_fake.py Expands unit coverage for payload-bearing classification (provider payload, trace stub vs real state, metadata error_message) and cross-checks behavior against a real Langfuse SDK client.
tests/conformance/test_observability.py Moves fixture 158 from deferred to activated in the Langfuse harness fixture set.
tests/conformance/test_observability_langfuse.py Extends the isolation runner to cover fixture 158, adds per-case deferral reporting via RecognizedSkip, implements payload-bearing leak assertions, and tightens directive allowlists per runner.
tests/conformance/harness/langfuse_real_client.py Adds prime_credential_on() to model “preexisting same-key client” behavior using the real SDK cache.
tests/conformance/harness/langfuse_provider_fake.py Renders metadata in a v4-faithful flattened shape and adds span_is_payload_bearing() / payload_bearing_spans() plus trace-stub detection.
conformance.toml Marks proposal 0115 as implemented and updates proposal notes to reflect fixture 158 activation and the implemented payload-bearing classification rules.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

tests/conformance/test_observability_langfuse.py:3125

  • This checks only that at least one matching decision record exists, but the contract and the comment above require exactly one WARNING. Emitting the warning twice (or emitting an additional level from the same decision site) would still pass. Compare the complete set of records from _ISOLATION_DECISION_FUNC with the expected levels so cardinality is enforced too.
            matched = [
                r for r in log_records if r.levelname == level and r.funcName == _ISOLATION_DECISION_FUNC
            ]

@chris-colinsky
chris-colinsky merged commit a1b7a1e into main Aug 15, 2026
7 checks passed
@chris-colinsky
chris-colinsky deleted the feature/0116-fixture-158-payload-leak branch August 15, 2026 01:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants