fix(webchat): reject remote-host file:// URLs in media embedding path [AI-assisted]

[pgondhi987]

Summary

• Problem: resolveLocalMediaPathForEmbedding in chat-webchat-media.ts used raw fileURLToPath() to resolve file: URLs, skipping the host validation that already exists for normal reply media paths. A file://attacker/share/probe.mp3-style URL could reach statSync/readFileSync; on Windows it resolves to a UNC path and can trigger outbound SMB access.
• Why it matters: Tool-result media in webchat bypassed an existing policy enforced for normal replies (reply-media-paths.ts already rejects host file:// URLs). This was a concrete bypass of a defense-in-depth boundary, with elevated impact on Windows deployments.
• What changed: Replaced fileURLToPath() with safeFileURLToPath() from src/infra/local-file-access.ts (validates hostname is empty/localhost, rejects encoded separators, rejects Windows UNC paths). Added assertNoWindowsNetworkPath() on the raw absolute-path branch to cover Windows UNC strings passed directly.
• What did NOT change: No behavior change for legitimate local file:///absolute/path or file://localhost/absolute/path inputs. No changes to routing, tool-result promotion logic, or other media paths.

Change Type (select all)

• Bug fix
• Security hardening

Scope (select all touched areas)

• Gateway / orchestration

Linked Issue/PR

• This PR fixes a bug or regression

Root Cause (if applicable)

• Root cause: resolveLocalMediaPathForEmbedding called fileURLToPath() directly without checking the URL hostname, while the existing safeFileURLToPath() helper (already used elsewhere) performs that validation.
• Missing detection / guardrail: No unit test existed for remote-host file:// URLs in the webchat media embedding path.
• Contributing context (if known): The tool-result media promotion path does not apply the same normalization as normal reply media (reply-media-paths.ts), so the weaker sink was reachable.

Regression Test Plan (if applicable)

• Coverage level that should have caught this:
  • Unit test
• Target test or file: src/gateway/server-methods/chat-webchat-media.test.ts
• Scenario the test should lock in: file://attacker/share/probe.mp3 returns zero audio blocks and never calls statSync or readFileSync.
• Why this is the smallest reliable guardrail: Directly exercises buildWebchatAudioContentBlocksFromReplyPayloads with the attack input and asserts no filesystem access occurs.
• Existing test that already covers this (if any): None — added in this PR.

User-visible / Behavior Changes

None. Legitimate local audio embedding is unaffected.

Diagram (if applicable)

Before:
[tool result: file://attacker/share/probe.mp3] -> fileURLToPath() -> statSync/readFileSync (UNC access on Windows)

After:
[tool result: file://attacker/share/probe.mp3] -> safeFileURLToPath() -> throws (remote host rejected) -> null -> dropped

Security Impact (required)

• New permissions/capabilities? No
• Secrets/tokens handling changed? No
• New/changed network calls? No — this PR removes an unintended one (outbound UNC/SMB on Windows)
• Command/tool execution surface changed? No
• Data access scope changed? Yes — filesystem access from webchat media embedding is now restricted to localhost-origin file:// URLs only
• Risk + mitigation: The change narrows access scope; no legitimate use case requires reading files from remote UNC hosts.

Repro + Verification

Environment

• OS: Linux (primary); Windows impact documented in advisory
• Runtime/container: Node 22+
• Model/provider: N/A
• Integration/channel: Webchat / Control UI

Steps

1. Call buildWebchatAudioContentBlocksFromReplyPayloads with mediaUrl: "file://attacker/share/probe.mp3".
2. Before fix: fileURLToPath passes through (or resolves to UNC on Windows); statSync is called.
3. After fix: safeFileURLToPath throws on non-localhost hostname; function returns [].

Expected

• Zero audio content blocks returned; no filesystem calls made.

Actual (after fix)

• Zero audio content blocks; statSync and readFileSync spy assertions confirm no filesystem access.

Evidence

• Failing test/log before + passing after — new test drops tool-result file:// URLs with remote hosts before touching the filesystem in chat-webchat-media.test.ts covers the exact attack vector.

Human Verification (required)

• Verified scenarios: Remote-host file:// URL rejected before filesystem access; existing tests for legitimate local paths continue to pass.
• Edge cases checked: file://localhost/... still resolves correctly; oversized files still rejected via stat cap; non-audio extensions still filtered.
• What you did not verify: Live Windows UNC SMB behavior (platform not available in review environment).

Review Conversations

• I replied to or resolved every bot review conversation I addressed in this PR.
• I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

• Backward compatible? Yes
• Config/env changes? No
• Migration needed? No

Risks and Mitigations

• Risk: Legitimate use of file://hostname/... URLs in webchat media (e.g., internal NFS mounts addressed via hostname).
  • Mitigation: No evidence this pattern is used or supported; the existing reply-media-paths.ts policy already blocks it for normal replies, making this consistent rather than restrictive.

[greptile-apps]

Greptile Summary

This PR fixes a security bypass in resolveLocalMediaPathForEmbedding where fileURLToPath() was called directly on file: URLs without host validation, allowing file://attacker/share/probe.mp3-style inputs to reach statSync/readFileSync (and trigger outbound SMB/UNC access on Windows). The fix replaces the raw call with safeFileURLToPath() — which already validates hostname, encoded separators, and Windows network paths — and adds assertNoWindowsNetworkPath() on the raw absolute-path branch. A targeted unit test verifies no filesystem access occurs for the attack input.

Confidence Score: 5/5

Safe to merge — tightly scoped security hardening with no behavior change for legitimate local paths, consistent with existing policy in reply-media-paths.ts.

All remaining findings are P2 or lower. The fix is minimal, uses the already-audited safeFileURLToPath helper, and is backed by a targeted test that confirms no filesystem access for the attack input. No logic changes outside the security boundary.

No files require special attention.

_{Reviews (1): Last reviewed commit: "fix: address issue" | Re-trigger Greptile}

[pgondhi987]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Hooray!

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b429dfb3cf

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[pgondhi987]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Another round soon, please!

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".