From d0d4b9844c00e8a7b15c99a6a70cbda554a9d648 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 21 Jul 2026 08:14:31 +0000 Subject: [PATCH 1/2] fix(security): sanitize user-controlled values in API logs (CWE-117 log injection) Rebuilt on current main. Adds a `_safe_log()` CR/LF scrubber and applies it to every user-controlled value interpolated into a v1-router log line, closing log-injection (CWE-117): a newline in a path param or request field could otherwise forge additional log entries. Sinks sanitized (14): chat `request.message`/`request.session_id`; video-context, video-not-found, and processing-complete `video_id`; video/markdown/ video-to-software `request.video_url`; action-retrieval `video_id`; action-update `action_id`; `job.job_id` (persist), `job_id` (cloud-tasks fallback, missing-at-runtime, job-failed); and `execution.agent_id`. An AST scan over the router confirms zero unsanitized user-controlled logger sinks remain. Adds `TestSafeLog` regression tests. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01FZcDgrGTkknC2ya13Uy6bU --- .../backend/api/v1/router.py | 38 ++++++++++++------- tests/unit/test_v1_router_extended.py | 17 +++++++++ 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/src/youtube_extension/backend/api/v1/router.py b/src/youtube_extension/backend/api/v1/router.py index b3b4cbc65..d8a6d09e4 100644 --- a/src/youtube_extension/backend/api/v1/router.py +++ b/src/youtube_extension/backend/api/v1/router.py @@ -110,6 +110,16 @@ logger = logging.getLogger(__name__) +def _safe_log(value: object) -> str: + """Strip CR/LF from a value before it enters a log line. + + Guards against log-injection (CWE-117): user-controlled inputs (path + params, request fields) can smuggle newlines to forge additional log + entries. Sanitize such values wherever they are interpolated into logs. + """ + return str(value).replace("\r", "").replace("\n", "") + + async def _emit_event(event_type: str, data: dict, subject: str | None = None) -> None: """Emit a CloudEvent if the publisher is available.""" @@ -572,7 +582,7 @@ async def chat_v1( """Chat endpoint with AI processing via AgentOrchestrator""" try: logger.info( - f"Chat request received: {request.message[:50]}... session={request.session_id}" + f"Chat request received: {_safe_log(request.message[:50])}... session={_safe_log(request.session_id)}" ) params = { @@ -593,13 +603,13 @@ async def chat_v1( video_id = match.group(1) if video_id: - logger.info(f"Adding video context for video_id: {video_id}") + logger.info(f"Adding video context for video_id: {_safe_log(video_id)}") detail = data_service.get_video_detail(video_id) # If video not found, trigger real-time processing if not detail and request.video_url: logger.info( - f"Video not found for {video_id}, triggering real-time processing" + f"Video not found for {_safe_log(video_id)}, triggering real-time processing" ) try: proc_result = ( @@ -609,7 +619,7 @@ async def chat_v1( ) if proc_result and proc_result.get("status") == "success": detail = data_service.get_video_detail(video_id) - logger.info(f"Real-time processing complete for {video_id}") + logger.info(f"Real-time processing complete for {_safe_log(video_id)}") except Exception as e: logger.error(f"Real-time video processing failed: {e}") @@ -674,7 +684,7 @@ async def process_video_v1( ): """Basic video processing endpoint""" try: - logger.info(f"Video processing request: {request.video_url}") + logger.info(f"Video processing request: {_safe_log(request.video_url)}") await _emit_event( "com.eventrelay.video.received", {"url": request.video_url}, @@ -743,7 +753,7 @@ async def process_video_markdown_v1( health_service.increment_metric("process_video_markdown_total") try: - logger.info(f"Markdown processing request: {request.video_url}") + logger.info(f"Markdown processing request: {_safe_log(request.video_url)}") result = await video_processing_service.process_video_for_markdown( request.video_url, request.force_regenerate @@ -779,7 +789,7 @@ async def video_to_software_v1( ): """Convert YouTube video to deployed software""" try: - logger.info(f"Video-to-software request: {request.video_url}") + logger.info(f"Video-to-software request: {_safe_log(request.video_url)}") target_info = resolve_deployment_target(request.deployment_target) result = await video_processing_service.process_video_to_software( @@ -1023,7 +1033,7 @@ async def get_actions_by_video_v1(video_id: str): actions = repo.get_by_video_id(video_id) return actions except Exception as e: - logger.error(f"Error retrieving actions for {video_id}: {e}", exc_info=True) + logger.error(f"Error retrieving actions for {_safe_log(video_id)}: {e}", exc_info=True) raise HTTPException(status_code=500, detail="Internal server error") @@ -1071,7 +1081,7 @@ async def update_action_v1(action_id: str, payload: dict[str, Any]): logger.debug("Action feedback recording failed", exc_info=True) return {"success": bool(success)} except Exception as e: - logger.error(f"Error updating action {action_id}: {e}", exc_info=True) + logger.error(f"Error updating action {_safe_log(action_id)}: {e}", exc_info=True) raise HTTPException(status_code=500, detail="Internal server error") @@ -1327,7 +1337,7 @@ def _sync_persist(): data = job.model_dump(mode="json") get_job_store().save(job.job_id, data) except Exception as exc: - logger.warning("Job persist failed for %s: %s", job.job_id, exc) + logger.warning("Job persist failed for %s: %s", _safe_log(job.job_id), exc) # If we are in an async loop, offload serialization and I/O to a thread try: @@ -1424,7 +1434,7 @@ async def _queue_transcript_action_job( except Exception as exc: logger.info( "Cloud Tasks unavailable for %s, using local background task: %s", - job_id, + _safe_log(job_id), exc, ) asyncio.create_task( @@ -1513,7 +1523,7 @@ async def _run_video_job( """Background coroutine that drives the transcript-action workflow.""" job = _load_video_job(job_id) if job is None: - logger.error("Video job %s missing at run time", job_id) + logger.error("Video job %s missing at run time", _safe_log(job_id)) return try: job.status = JobStatus.downloading @@ -1559,7 +1569,7 @@ async def _run_video_job( job.status = JobStatus.failed job.error = str(exc) _persist_video_job(job) - logger.error(f"Video job {job_id} failed: {exc}") + logger.error(f"Video job {_safe_log(job_id)} failed: {exc}") @router.post( @@ -2089,7 +2099,7 @@ async def _run_agent(execution: AgentExecution, events: list[dict[str, Any]]): except Exception as exc: execution.status = AgentStatus.failed execution.error = str(exc) - logger.error(f"Agent {execution.agent_id} failed: {exc}") + logger.error(f"Agent {_safe_log(execution.agent_id)} failed: {exc}") @router.get( diff --git a/tests/unit/test_v1_router_extended.py b/tests/unit/test_v1_router_extended.py index cd484b1c3..7713691f1 100644 --- a/tests/unit/test_v1_router_extended.py +++ b/tests/unit/test_v1_router_extended.py @@ -2357,3 +2357,20 @@ def test_eviction_targets_least_recently_touched(self): d["d"] = 4 # overflow -> evict b assert "b" not in d assert {"a", "c", "d"} <= set(d.keys()) + + +class TestSafeLog: + """Regression guard for log-injection (CWE-117) sanitization.""" + + def test_safe_log_strips_crlf(self): + from youtube_extension.backend.api.v1 import router as router_module + + malicious = "vid123\r\nERROR forged-admin-login-success" + scrubbed = router_module._safe_log(malicious) + assert "\n" not in scrubbed and "\r" not in scrubbed + assert scrubbed == "vid123ERROR forged-admin-login-success" + + def test_safe_log_coerces_non_str(self): + from youtube_extension.backend.api.v1 import router as router_module + + assert router_module._safe_log(42) == "42" From 0f060278cefb25f93385bee45d553793a216d575 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 21:38:06 +0000 Subject: [PATCH 2/2] docs(triage): PR remediation run 2026-07-27 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Record terminal states for all 36 open PRs. #932 merged (by controller); 6 non-draft PRs HALTED on human/protected-environment gates (no automerge label, main protected); 28 drafts + #1043 DEFERRED. No merge performed — publish gate is human per runbook. No autonomous work remains. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01RfguqwFFVth5xwZC1Bx39X --- docs/triage/pr-remediation-2026-07-27.md | 62 ++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 docs/triage/pr-remediation-2026-07-27.md diff --git a/docs/triage/pr-remediation-2026-07-27.md b/docs/triage/pr-remediation-2026-07-27.md new file mode 100644 index 000000000..000916426 --- /dev/null +++ b/docs/triage/pr-remediation-2026-07-27.md @@ -0,0 +1,62 @@ +# PR Remediation Run — 2026-07-27 + +**Runbook:** PR Remediation & Publish Runbook (action-forcing, RASOR DAG). +**Surface:** GitHub MCP (read + write capable). +**Scope:** `groupthinking/eventrelay` only. +**Policy in effect:** `auto_merge_policy: label:automerge` · `main` is protected · publish gate is human. + +## Definition-of-Done outcome + +Every open PR reached a terminal state. **No PR was merged by this run** — this is +correct, not a stall: zero PRs carry the `automerge` label, `main` is protected, and +every non-draft PR's own body documents human-gated merge blockers (protected +staging/production proofs, historical-provenance disposition, final human review, +explicit "do not merge"). Runbook §8 routes each of these to +`HALTED(awaiting_merge_approval)` — the merge is the irreversible, human-signed step +this run must not bypass. + +An active `eventrelay-blocker-watch` controller is reconciling these PRs live +(all heads updated within minutes of this scan; #932 merged during it). Re-running +the CodeRabbit review loop would duplicate that controller and spend review allowance +on PRs that are already human-blocked, so it was intentionally not triggered. + +## Entry scan + terminal states (§6 output contract) + +| PR | Author | Age | Review | CI (per exact-head evidence) | Conflicts | Action taken | Terminal state | +|----|--------|-----|--------|------------------------------|-----------|--------------|----------------| +| #932 | mirkosalvato1-ctrl | — | resolved | green | none | merged by controller during scan | **MERGED** | +| #734 | groupthinking | 15d | 0 unresolved | green | mergeable_state=unknown | observed; no safe transition | **HALTED**(provenance + final human review) | +| #810 | groupthinking | 10d | 0 unresolved | unstable (a check pending/failing) | — | observed; no safe transition | **HALTED**(provenance + final human review) | +| #831 | groupthinking | 10d | 0 unresolved | green | mergeable_state=unknown | observed; no safe transition | **HALTED**(current-head review pending + provenance) | +| #869 | groupthinking | 9d | 0 unresolved | green | — | observed; body says "do not merge" | **HALTED**(protected staging/prod durability proof) | +| #903 | google-labs-jules[bot] | 7d | 0 unresolved | non-authoritative coverage | — | observed | **HALTED**(protected production credential + sign-in verification, #900) | +| #906 | groupthinking | 6d | 13/13 resolved | green except E2E fail-closed (missing preview-bypass secret) | — | observed | **HALTED**(protected staging migration + E2E preview credential) | +| #1043 | google-labs-jules[bot] | 0d | — | green (tree-empty commits) | — | observed; controller marked duplicate | **DEFERRED**(duplicate of #1020/#1022) | +| 28 draft PRs | various | — | — | — | — | scope gate (§2) | **DEFERRED**(draft) | + +Draft PRs deferred: 948, 961, 973, 980, 983, 987, 990, 994, 995, 996, 997, 999, +1000, 1001, 1002, 1003, 1004, 1005, 1006, 1007, 1008, 1009, 1020, 1022, 1038, 1040, +1044, 1045. + +## Staged next command per HALTED PR (human-gated — do NOT auto-run) + +The blocker on all six HALTED PRs is a human/protected-environment gate, not a +resolvable code or CI defect. Merge is staged but withheld pending sign-off: + +``` +# only after the named human gate clears for each PR: +#734 gh pr merge 734 --squash # after legacy-provenance disposition + human review +#810 gh pr merge 810 --squash # after unstable check clears + provenance + human review +#831 gh pr merge 831 --squash # after current-head automated review + provenance +#869 gh pr merge 869 --squash # after protected staging/prod durability proof +#903 gh pr merge 903 --squash # after protected Vercel OAuth creds verified + real sign-in (#900) +#906 gh pr merge 906 --squash # after protected staging migration + E2E preview credential +``` + +## Is more work needed? — No autonomous work remains + +All open PRs are in a determined terminal state. Every remaining transition requires +a human decision or a protected-environment proof the runbook explicitly forbids +automating, and a live controller is already handling exact-head reconciliation. The +dynamic loop is therefore stopped; the six HALTED PRs are handed to a human with the +exact blocker and staged merge command above.