fix(security): neutralize CWE-117 log forging in StructuredFormatter - #1263
fix(security): neutralize CWE-117 log forging in StructuredFormatter#1263groupthinking wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
CI triage (head
|
| Check | Cause | Disposition |
|---|---|---|
label |
PR had no qualifying label | Fixed — added python, security, tests (accurate to the diff); re-triggers the gate |
validate / Canonical issue and evidence |
GitHub API rate-limit 403 (x-ratelimit-remaining: 0, resets ~18:32 UTC) — transient infra, not the diff |
Clears on re-run after reset |
dependency-review |
Repo-level: "Dependency review is not supported… ensure Dependency graph is enabled" — fails on every PR, and this diff changes no dependencies | Not fixable in-PR; needs repo Settings → Security |
agent-completion/truth-gate, Agent completion enforcement |
Provenance gate needs a frozen pre-dispatch intent + trusted terminal agent result | Human/owner gate — see below |
Code status is green: focused suite tests/unit/test_logging_config_crlf.py = 7 passed, and the exact-head Vercel preview is Ready. The CWE-117 fix and the two Sentinel code-gen fixes are complete and verified.
Genuine blocker (unattended-run honest disclosure): this was produced by a scheduled, non-interactive pass. I'm not authorized to merge to protected main autonomously, and the agent-completion/truth-gate requires provenance an unattended run can't supply. So this PR is HALTED(awaiting_owner) — the remaining work is owner sign-off + merge, not more code.
Staged next command for the owner once approved:
gh pr merge 1263 --repo groupthinking/EventRelay --squash
Generated by Claude Code
Scrub CR/LF and other line separators (VT, FF, NEL, U+2028, U+2029) from the fully rendered log record in StructuredFormatter.format(). Because the scrub runs on the final formatted string, it covers the message, exc_info traceback text (including str(exc)), and structured `extra` fields at once — closing the log-forging vector that inline message sanitizers miss at exc_info / logger.exception sinks. Separators are escaped rather than dropped, so content stays greppable while one logging call is guaranteed to render as one physical line. Adds tests/unit/test_logging_config_crlf.py asserting the *rendered* handler output (not a sanitizer return value) cannot be used to forge a standalone log line via message, exc_info traceback, or Unicode line separators. Addresses the confirmed unmitigated finding on #810 (CWE-117); this is the central formatter-level remediation that complements #810's per-sink message sanitization. Progresses #898. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KDtvQR6b5JXkAStGbTCTxL
689740a to
d88875d
Compare
groupthinking
changed the title
Update — de-duplicated against #1259 (head now
|
Agent Completion Truth Gate: BLOCKEDReasons: Machine-readable verdict{
"details": {
"invalid_fields": [
"policy.agent_login",
"policy.run_id"
]
},
"reasons": [
"invalid_payload"
],
"verdict": "blocked"
} |
Truth-gate
|
|
Closing as strictly dominated — no unique coverage. Empirically compared against the other five PRs in this CWE-117 cluster by pushing every control/separator codepoint through Full comparison matrix and the consolidation plan: #1270 (comment). Reopen if I have missed a non-coverage reason to keep this branch. |
2 participants
Canonical issue
Progresses #898 (CWE-117 log-injection hardening). Addresses the confirmed, previously-unmitigated
exc_infofinding raised on #810 — this is the central, formatter-level remediation that complements #810's per-sink message sanitization (not competing: #810 scrubs individualrouter.pysinks; this scrubs the final rendered record for every logger).Outcome
A user-controlled value containing
\r\n(or other line separators) can no longer forge or corrupt a log line — not via the message, not via anexc_infotraceback, and not via structuredextrafields. One logging call renders as exactly one physical line.Scope
src/youtube_extension/backend/config/logging_config.py— scrub CR/LF, VT, FF, NEL, U+2028, U+2029 from the fully rendered record inStructuredFormatter.format().tests/unit/test_logging_config_crlf.py— new regression suite (asserts rendered handler output).router.pyper-sink sanitization (fix(security): sanitize user-controlled values in API logs (CWE-117 log injection) #810's scope);code_generator.pyinfo-disclosure (🛡️ Sentinel: [MEDIUM] Fix information disclosure in code generator #1259's scope).Risk
\n). Content is fully preserved and greppable; separators are escaped, not dropped.logging_config.pyhunk; formatter reverts to prior passthrough behavior.Verification
tests/unit/test_logging_config_crlf.py: 7 passed on headd88875d(message forging,exc_infotraceback forging, and 5 Unicode/control separators), asserted against rendered handler output.d88875d.Standalone evidence:
Production evidence
Not applicable to runtime behavior: Python-only logging-format change. Vercel builds the Next.js
apps/webroot and does not exercise this path (exact-head preview was Ready on the prior revision).Agent handoff
mainAgent provenance