fix(security): CWE-117 JSON field forgery via unescaped `"` survives the #1270 log sanitizer

[groupthinking]

Summary

The CWE-117 hardening merged in #1270 (55e0e64, ~17:55 today) neutralizes every line/record separator but not the double-quote. With enable_json_logging on, JSON records are assembled by printf-style interpolation, so attacker-controlled " breaks out of the string value and forges arbitrary JSON fields.

Found by the Vercel VADE reviewer on #1423 and verified against main before filing.

Reproduction — verified, not theoretical

Attacker content contains no newline and no backslash — only a ", which the escape table does not cover.

attacker = 'benign", "level": "DEBUG", "forged": "yes'
log.info(attacker)

Rendered record:

{"timestamp": "...", "service": "youtube-extension-api", "version": "2.0.0",
 "level": "INFO", "logger": "repro", "message": "benign", "level": "DEBUG",
 "forged": "yes", "module": "repro_quote.py", "line": 28, ...}

Parsed:

PARSED level   : DEBUG
PARSED message : benign
PARSED forged  : yes

*** FORGED: record was emitted at INFO but parses as DEBUG
*** INJECTED: attacker added a field the format string never defined

The record was emitted at INFO and parses as DEBUG. On a duplicate key most JSON parsers take the last value, so the attacker's level wins. An attacker can downgrade their own entries below an alerting threshold and inject fields downstream consumers trust.

This is the exact outcome #1270 set out to prevent. It stays parseable JSON — which is what the code comment claims — but parseable is not the same as faithful, and the comment at logging_config.py:25-28 ("stays valid JSON") reads as a stronger guarantee than it delivers.

The one-line fix does not work

Adding ord('"'): "\\u0022" to _UNSAFE_LOG_CHARS breaks JSON logging outright. Tested on a benign message:

{"timestamp": "2026-08-07 18:17:27", "service": ...

RESULT: JSON IS NOW BROKEN -> Expecting property name enclosed in double quotes: line 1 column 2

The cause is structural. StructuredFormatter.format() sanitizes the fully rendered record (logging_config.py:83-89):

formatted_message = super().format(record)
return sanitize_log_record(formatted_message)

By then, attacker content and the template's own structural quotes are the same characters. The table cannot distinguish them:

• Leave " unescaped → field forgery (the live bug).
• Escape " → the JSON skeleton is destroyed.

So "escape the rendered record" cannot be made correct for JSON. This is a design limit of the approach, not a missing table entry.

Suggested direction

Escaping must happen before interpolation, so only field values are transformed:

1. Build the record with json.dumps over a dict when enable_json_logging is on. Correct escaping for free, and structure can never be forged. Preferred.
2. Or sanitize per-field (record.msg, record.args, exc_info text, extra) and let the format string interpolate already-safe values, leaving the template's quotes untouched.

Either keeps the existing separator neutralization; the change is where it is applied. The line-oriented (non-JSON) paths are unaffected and can keep the current behavior.

Exposure

src/youtube_extension/backend/config/production_config.py:72 defaults JSON_LOGGING to "true":

self.enable_json_logging = os.getenv("JSON_LOGGING", "true").lower() == "true"

(logging_config.py:313 defaults it to "false" — worth reconciling separately; the two disagree.) Any log call that renders untrusted input into %(message)s is a sink.

Acceptance criteria

• A " in attacker-controlled content cannot introduce, terminate, or duplicate a JSON field.
• level, timestamp, and logger reflect what the logger actually emitted, regardless of message content.
• Benign messages still produce valid, parseable JSON — the naive fix's regression is covered by a test.
• Existing separator neutralization (LF, CR, VT, FF, FS, GS, RS, NEL, LS, PS, ESC) and reversibility are preserved.
• The logging_config.py:25-28 comment states the guarantee the code actually provides.

Notes

Not folded into #1423, which is scoped to the Dependabot governance gate (#1419) and carries its own single-issue contract — the same reason #1270 dropped an unrelated perf commit from its branch.

[linear-code]

GRV-348

[github-actions]

👋 Thanks for opening this issue! It has been automatically labeled for triage.

[coderabbitai]

🔗 Related PRs

#605 - fix(follow-up): correct Trivy commit pin + broaden YouTube host allowlist (CodeRabbit findings on merged #604) [merged]
#693 - fix(ci): unblock the entire PR backlog — resolve invalid agent_network.json + restore apps/web type-check script [merged]

---

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

• Create Plan

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!