fix(crewai): stop the __dict__ walk leaking credentials onto spans - #4465
fix(crewai): stop the __dict__ walk leaking credentials onto spans#4465IdoGol24 wants to merge 2 commits into
Conversation
CrewAISpanAttributes stringified every attribute of the Crew, Agent and Task objects it was handed, so anything those objects held -- an LLM client, an embedder config -- reached the span through its repr, api keys included. Read a fixed set of fields instead. Agents already had a hand-written allowlist in _extract_agent_data; the standalone-agent path just wasn't using it. Tasks reuse the dict _parse_tasks already built. Only Crew needed a new list. Tools go through the existing _serialize_tools on every path rather than being repr'd, and the dead _populate_llm_attributes is gone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012SsveLwpjmyX6GEydCuX6H
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughCrewAI span extraction now uses an explicit safe-field allowlist and shared serialization for agents, tasks, and tools. New tests verify that sentinel credentials do not enter span attributes while identifiers remain available. ChangesCrewAI attribute safety
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to CrewAI span extraction now limits serialized fields to safe allowlists to prevent credential leakage while retaining supported identifiers. No concrete merge-blocking risk remains. Sequence Diagram(s)sequenceDiagram
participant CrewInstance
participant CrewAISpanAttributes
participant InMemorySpanExporter
CrewInstance->>CrewAISpanAttributes: provide crew, agent, and task data
CrewAISpanAttributes->>CrewAISpanAttributes: select allowlisted fields and serialize tools
CrewAISpanAttributes->>InMemorySpanExporter: emit filtered span attributes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Satisfies the docstring coverage check on the PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012SsveLwpjmyX6GEydCuX6H
CrewAISpanAttributesstringified every attribute of the Crew, Agent and Task objects it was handed, so anything those objects held -- an LLM client, an embedder config -- reached the span through its repr, api keys included.Read a fixed set of fields instead.
Agentsalready had a hand-written allowlist in _extract_agent_data; the standalone-agent path just wasn't using it. Tasks reuse the dict _parse_tasks already built. Only Crew needed a new list. Tools go through the existing_serialize_toolson every path rather than being repr'd, and the dead_populate_llm_attributesis gone.I have added tests that cover my changes.
If adding a new instrumentation or changing an existing one, I've added screenshots from some observability platform showing the change.
PR name follows conventional commits format:
feat(instrumentation): ...orfix(instrumentation): ....(If applicable) I have updated the documentation accordingly.
Summary by CodeRabbit