[SAR] APPENG-5151: Configurable audit logging

[yuvalk]

Summary

Addresses APPENG-5151.

Add an AUDIT_LOGGING_ENABLED setting that controls whether PII fields (user_id, org_id, order_id, request_id) are injected into log records. This allows operators to disable PII in logs for compliance while maintaining consistent log format structure.

Changes

• Add audit_logging_enabled setting to config/settings.py (default: True)
• Update AuditContextFilter in logging/filters.py to check the setting before injecting context
• When disabled, PII fields are set to empty strings to preserve JSON log schema consistency

SAR Reference

• CWE: CWE-532 (Insertion of Sensitive Information into Log File)
• Impact: Medium

Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com

[luis5tb]

PR 202 Review: [SAR] APPENG-5151 — Configurable Audit Logging

Critical (1)

• Toggle only suppresses PII in the log filter, not in direct log messages — AUDIT_LOGGING_ENABLED=False controls AuditContextFilter but AgentLoggingPlugin._audit_fields() (api/a2a/logging_plugin.py:55-62) independently reads and logs user_id, org_id, order_id, request_id from contextvars in 8+ log statements across every tool call, LLM invocation, and run lifecycle event. Similarly, mcp_headers.py:48-55 logs user_id, org_id, request_id on every JWT forwarding event. An operator who disables the toggle expecting no PII in logs will still see PII throughout. → Gate _audit_fields() and mcp_headers.py on the same setting, or explicitly scope the toggle in docs as "structured log record fields only." [logging/filters.py:21, api/a2a/logging_plugin.py:54-62, tools/mcp_headers.py:48-55]

Important (2)

• Missing tests for disabled mode — No tests exercise audit_logging_enabled=False. Existing TestAuditContextFilter only covers the default (enabled) behavior. Add tests that patch get_settings().audit_logging_enabled to False and assert all record fields are empty strings even when contextvars are populated. [tests/test_audit_logging.py]

• Missing .env.example update — The new AUDIT_LOGGING_ENABLED env var is not documented in .env.example, which has a dedicated "Audit logging" section (lines 173-184). Add it with a comment explaining the PII implications. [.env.example:173]

Suggestions (3)

• Per-log-record deferred import overhead — filter() does from lightspeed_agent.config import get_settings + get_settings() on every log record. While both are cached (sys.modules + lru_cache), consider caching the boolean at construction time since Settings is a singleton. [logging/filters.py:22-23]

• Consider a startup warning when audit logging is disabled — Unlike SKIP_JWT_VALIDATION which has a production guard via _block_skip_jwt_in_production, this setting has no guardrail or visibility. A WARNING at startup when disabled would help operators notice misconfiguration. [config/settings.py]

• Cross-service consistency — Agent (port 8000) and Marketplace Handler (port 8001) have independent settings. If one disables audit logging and the other doesn't, PII leakage is inconsistent. Document that AUDIT_LOGGING_ENABLED should be set identically across both services. [config/settings.py]

Strengths

• Empty-string default preserves JSON schema — downstream log consumers won't break
• Setting is well-placed in the Logging section with a clear description of the trade-off
• Default True preserves backward compatibility
• Clean, minimal diff — does exactly what it says
• PR description is thorough with SAR reference and CWE citation

[luis5tb]

fixes to the reveiew at: yuvalk#7