Repository navigation
[1850] fix: Resolve security audit findings: secret redaction, trace_content gating and instrumentation crashes - #420
Open
RiyaSunil27 wants to merge 28 commits into
Open
RiyaSunil27 wants to merge 28 commits into
RiyaSunil27 wants to merge 28 commits into
Conversation
RiyaSunil27
force-pushed
the
develop
branch
3 times, most recently
from
October 15, 2026 14:12
e4c5149 to
d09d020
Compare
akash-vijay-kv
force-pushed
the
develop
branch
2 times, most recently
from
November 2, 2026 12:54
5ea55bf to
6349dbf
Compare
RiyaSunil27
force-pushed
the
fix/security-audit
branch
from
November 4, 2026 07:03
4fe10d2 to
3c2c802
Compare
RiyaSunil27
changed the title
…ns to debug any issue related to audio deliver (#404)
…teardown in _ShieldedTracerProvider (#419)
…al lk.* keys (#432) * [NET-968] refactor: Remove pii detection & prompt injection detection modules (#330) * [NET-1590] fix: Add a span audio.delivery.metadata in the livekit spans to debug any issue related to audio deliver (#404) * [3464] fix: Ensure shutdown flushes spans before absorbing LiveKit's teardown in _ShieldedTracerProvider (#419) * [#1981] fix: Map LiveKit ≥1.7 lk.pii.* attributes alongside the original lk.* keys --------- Co-authored-by: Akash Vijay <akash@keyvalue.systems>
akash-vijay-kv
force-pushed
the
develop
branch
from
November 4, 2026 08:56
67e201c to
86b91e7
Compare
akash-vijay-kv
changed the title
The email pattern backtracked quadratically on inputs like "a.a.a...", so a crafted 50k-character attribute stalled span.end() for about 20 seconds when scrubbing was enabled. Removing the "|" typo alone does not change this. Matches now start only at the beginning of a local part and every repeated segment is length-bounded. Also drops the literal "|" from the TLD class.
A NETRA_API_KEY ending in a newline (common with Kubernetes secrets and .env files) was rejected by the HTTP client, and the upstream exporter then logged the full key on every failed batch while all telemetry was dropped. The key and every header value are now stripped. A value that still contains a control character is dropped with a warning that names the setting but never shows the value. The caller's headers dict is no longer mutated.
x-goog-api-key, x-amz-security-token, ocp-apim-subscription-key and the CSRF token headers were exported verbatim because only seven names were redacted. Adds those names, and also redacts any header with a whole dash-separated part of key, token, secret, password, signature or credential. Whole-part matching (not substring) keeps debugging headers such as x-ratelimit-remaining-tokens visible; idempotency-key and sec-websocket-key are exempt. The same rule now applies to header attributes recorded by upstream OpenTelemetry instrumentations.
Only user:pass@ was stripped from URLs, so ?key=, presigned S3/GCS/Azure signatures and OAuth code/access_token values were exported in http.url and input.url. Inbound FastAPI and Agno query strings were not stripped at all. Adds redact_url_secrets / redact_query_secrets, which replace the values of deny-listed query and fragment parameters (case-insensitive) while keeping order, encoding and blank values; a URL with nothing to redact is returned unchanged. Applied in httpx, requests, aiohttp, FastAPI, Agno, Deepgram and to upstream http.url / url.full attributes in the instrumentation processor.
The LM wrapper exported the merged lm.kwargs and call kwargs as llm.headers, so dspy.LM(..., api_key=...) put the provider key (and api_base) on the span. The call-time prompt and any api_key argument also reached llm.headers and gen_ai.request.input. llm.headers now holds only an allowlist of invocation parameters (model, temperature, max_tokens, top_p, n, stop, penalties, seed, response_format), and credential-like keys are dropped recursively from the exported inputs.
Not an audit finding; found while fixing H-3. ToolCallWrapper printed every tool call's keyword arguments to stdout, including any credentials or user data passed to the tool.
…H-15) json.dumps(reasoning) raised TypeError for NOT_GIVEN, omit and Reasoning(...) values. The openai-agents SDK passes omit on every call, so with Netra installed every Responses API call failed before the request was sent. Request attributes now go through to_jsonable (unset sentinels are skipped, pydantic models are dumped) and the OpenAI wrappers record them in a guarded helper, so only exceptions from the SDK call itself reach the caller. The same sentinel handling applies to the LiteLLM and Groq request mappings. llm_common.py also hosts the shared stream base that the later stream lifecycle fix (H-22) builds on.
…it H-16)
The completed-response handler iterated output.get("content") for every output
item. function_call and reasoning items have no content, so enumerate(None)
raised TypeError from inside the user's for loop, and each part overwrote the
previous one so only the last text survived.
Output items without content are now skipped, all text parts are concatenated,
function calls are exported as completions, and an "incomplete" status is
accepted like "completed".
The OpenAI, LiteLLM, Groq, Cerebras and Gemini stream wrappers were near copies that had drifted apart: a mid-stream error left an OpenAI span to be closed as OK by __del__, the others exported no span at all after an early break, an error or close(), and `with groq...create(stream=True) as s` handed back the raw SDK stream so nothing was traced. They now share BaseLLMStream: one idempotent finalizer, ERROR status plus re-raise on a mid-stream exception, close/aclose/__del__, and __enter__ / __aenter__ returning the wrapper. Chunk parsing is guarded so telemetry can never raise inside the user's loop. Also fixed here because they sit inside the rewritten wrappers and cannot be separated: Cerebras streaming no longer swallows SDK errors, Groq non-streaming no longer replaces the user's exception with UnboundLocalError, a null choice index no longer raises (M-24), and request and response attributes are recorded in guarded helpers in all five.
Not in the original audit; found while verifying H-17. In pydantic-ai 2.x `usage` is a property, so result.usage() raised "'RunUsage' object is not callable" from every instrumented agent.run() and run_sync(), failing the user's call after it had already succeeded. Usage is now read whether it is a method (old) or a property (2.x), token counts fall back across the old and new field names, and all response attribute extraction is wrapped so a change in result shape can never raise into the caller.
…t H-17) The run_sync wrapper never called the original: it built its own event loop, set it as the thread's loop and closed it, so later asyncio.get_event_loop() calls failed with "Event loop is closed". It also bypassed pydantic-ai's nested-run guard, agent name inference and upstream run_sync fixes. The wrapper now delegates to wrapped() inside its span; the run and iter wrappers emit the child spans. The agent name is inferred from the user's call site, and run() returns the real result type instead of a proxy. Behaviour change: run_sync now has one extra span level (run_sync -> run -> iter). Removes pydantic_ai_slim/wrappers.py and utils.py, which were unused copies: the slim instrumentor already imports from pydantic_ai.wrappers.
Not one of the listed findings; found while fixing H-17. unwrap(module, "Class.method") looked up a module attribute literally named "Class.method", so it silently did nothing and instrumenting again stacked a second set of wrappers (duplicate spans per call). The class is now resolved first and unwrapped by method name, for both the pydantic_ai and pydantic_ai_slim instrumentors.
…H-20) The function wrappers caught only Exception, so CancelledError, KeyboardInterrupt, SystemExit and GeneratorExit skipped span.end(), unregister_span and pop_entity. A cancelled @workflow exported no spans and left its root registered forever. pop_entity also ran inside use_span, whose context detach reverted it, so every ordinary failure left a stale task name that was stamped onto later spans in that thread. Failed spans were exported with UNSET status because end() ran before the status was set. Wrappers now catch BaseException and finalise after use_span exits: ERROR status plus the recorded exception before end() for real failures, an entity.cancelled marker (status left UNSET) for cancellation and generator close, and no error for SystemExit(0). The same handling covers the generator and StreamingResponse paths, and the Google ADK run wrapper now ends its span and detaches its context on aclose() or cancellation.
ElevenLabs, Agno and Deepgram stream wrappers attached the span to the current context and returned while still attached, detaching only on full consumption. After `for x in stream: break`, every later unrelated span in that thread or task was parented under the dead stream span, the stream span itself was never exported (ElevenLabs), and consuming a stream in another task logged "Failed to detach context". The factories now detach before returning and the stream wrappers attach the span around each __next__/__anext__ only. ElevenLabs gains an idempotent finalizer with close/aclose/__del__ (its unused chunk buffer is removed in the same rewrite), Agno's stream wrappers gain close(), and Deepgram no longer holds the context across yield. Spans a user creates between chunks are no longer accidentally children of the stream span.
set_session_id/user_id/tenant_id attach a context that is never detached, so
the value sticks to the thread. Servers and workers that reuse threads
(gunicorn/uWSGI sync workers, Celery, queue consumer loops) carried one
request's ids into the next, and an id set on request 2 could be combined with
a tenant left over from request 1. Passing "" was ignored, so a value could
not be cleared.
Adds Netra.session_scope(...), which applies the ids inside a block and
restores the previous values on exit (also when the block raises), and
Netra.clear_session(). Passing "" or None to a set_* method now clears just
that id. README documents session_scope as the pattern for servers.
Behaviour change: set_*("") used to log a warning and keep the old value.
Parallel uploads (four in flight) meant the circuit-breaker check, which ran only before waiting for a slot, was bypassed: with a rejected key, up to four requests went out, and a retry waiting in backoff never re-checked the circuit, so it was sent anyway. Uploads now run one at a time until the first 2xx proves the credential, then in parallel, so a rejected key costs exactly one request. Each retry attempt re-checks the circuit before sending.
Not one of the listed findings (audit M-25); found while fixing H-18. The session-end request was scheduled before the in-flight chunk uploads had finished, but the backend renders the session as soon as it arrives, so it could overtake chunks still on the wire and the audio would be rendered partial or wait on retries. The sender now waits for in-flight uploads before posting the session terminator, and the integration test records how many requests were answered before each arrived to prove it.
Not one of the listed findings (audit M-30). The test asserted that sequence numbers arrive in order, but uploads run in parallel so arrival order is not guaranteed; the backend stores chunks by sequence number and only needs 0..N with no gaps. The test now compares the sorted sequences.
…active (audit H-19) On current SDK majors these three produced no spans and no signal: Cohere 6.x and 7.x were rejected by the "<6" range, the Mistral 3.x namespace package never fired its "mistralai" trigger (and its module-scope type imports would have failed), and Cartesia 4.x moved the patched modules to cartesia.resources. Cohere's range is widened to <8. Mistral gets a 3.x trigger, SDK-type-free imports and stream handling, and a wrap table chosen by layout. Cartesia gets a 4.x table for cartesia.resources.* chosen by probing. An instrumentor that declines to activate, fails to activate, or finds no known layout now emits one RuntimeWarning naming the installed version and supported range. Also fixed in the rewritten Mistral/Cartesia registration code, which cannot be separated from it: uninstrument() now really unwraps (audit M-20), and Mistral response ids and choices are read with correct operator precedence.
Not an audit finding; found while fixing H-3 and H-4. The DSPy LM, module, predict, retriever, embedder and tool wrappers caught every exception, logged it and returned None, so a provider error made the user's call silently return None instead of raising. The wrappers now run the real call first, record the exception on the span with ERROR status and re-raise it. Failures while recording telemetry are logged at DEBUG and never reach the caller.
NETRA_TRACE_CONTENT=false only set TRACELOOP_TRACE_CONTENT, which only the Traceloop-based instrumentors and three of Netra's own (Cohere, Mistral, Hermes Agent) read. Prompts, completions, tool arguments and transcripts were still exported for OpenAI, LiteLLM, Groq, Cerebras, Gemini, Claude Agent SDK, pydantic-ai, DSPy, Agno, Google ADK, Honcho, LiveKit, ElevenLabs, Cartesia and Deepgram, for users who had turned capture off for compliance. Adds should_capture_content() / set_content_attribute() in span_utils, which read trace_content from the active config (falling back to the env var before init, and keeping Traceloop's override context value), and gates every prompt, completion, tool argument/result, system prompt, transcript and TTS/STT text attribute on it. Model, usage, cost, finish reasons, ids and status are never gated. The three existing local gates now use the shared helper. Not covered: HTTP body capture (httpx, requests, FastAPI; audit H-7) and the inputs/outputs of @workflow/@task decorators, which the user chooses.
akash-vijay-kv
force-pushed
the
fix/security-audit
branch
from
November 4, 2026 09:02
3c2c802 to
4f00369
Compare
akash-vijay-kv
force-pushed
the
develop
branch
2 times, most recently
from
November 5, 2026 08:42
1370838 to
7b3fbe6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue #1850