fix: stop fetch_url following redirects past the approval gate

[praisonai-triage-agent]

Fixes #4490

Problem

The desktop engine's fetch_url (src/praisonai-desktop/engine/server.py) gated on the approved url, then called urllib.request.urlopen(url) which follows any 3xx redirect to any host — including the engine itself on loopback. A page whose author (or an echoed prior tool result) controls a redirect could send the fetch to another chat's transcript, /settings, or /logs, and the body landed back in the model's context. No re-gate, no scheme check after the first hop — the approval card was a lie about what actually happened.

Fix

fetch_url now builds an opener with a HTTPRedirectHandler subclass that raises HTTPError on any redirect instead of following it. The honest error reaches the model, which can then ask the user for the new URL and get a second approval card. Minimal, no new params or dependencies.

Test

Added FetchUrlDoesNotFollowRedirects in engine/test_chat_stream.py: stands up two loopback ThreadingHTTPServers — A returns 302 Location: <B>, B returns SECRET-BODY — sets approval_mode: "never" so the gate allows, calls the fetch_url closure from _builtin_tools() on A's URL, and asserts the secret is not in the result. Asserts the effect (what came back), not that a check was called.

All 16 tests in engine/test_chat_stream.py pass.

Generated with Claude Code

[praisonai-triage-agent]

@coderabbitai review

[praisonai-triage-agent]

/review

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[greptile-apps]

Greptile Summary

The PR prevents the desktop engine’s fetch_url tool from automatically following an unapproved HTTP redirect.

• Adds a custom HTTPRedirectHandler that converts redirects into an actionable fetch error.
• Adds a two-server regression test proving content from the redirect target is not returned to the model.

Confidence Score: 5/5

The PR appears safe to merge and closes the redirect-based approval bypass without introducing an actionable regression.

The changed opener rejects every redirect before requesting its target, returns the resulting HTTP error through the existing tool contract, and the regression test verifies that target content does not reach the tool result.

Important Files Changed

Filename	Overview
src/praisonai-desktop/engine/server.py	Replaces automatic redirect following with a local no-redirect opener while preserving the existing tool result and error contract.
src/praisonai-desktop/engine/test_chat_stream.py	Adds an isolated end-to-end regression test demonstrating that content behind a redirect is not fetched.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    M[Model requests fetch_url] --> G{Original URL approved?}
    G -- No --> D[Return declined result]
    G -- Yes --> F[Fetch approved URL]
    F --> R{HTTP redirect?}
    R -- No --> B[Return sanitized response body]
    R -- Yes --> E[Return redirect-not-approved error]
    E --> N[Model may request target URL separately]
    N --> G

Loading

_{Reviews (1): Last reviewed commit: "fix: stop fetch_url following redirects ..." | Re-trigger Greptile}

[MervinPraison]

@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Review changes in this PR. Python SDK: praisonaiagents, praisonai. TypeScript SDK: src/praisonai-ts/. Do NOT modify src/praisonai-rust. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding.

Phase 1: Review per AGENTS.md

1. Protocol-driven: check heavy implementations vs core SDK
2. Backward compatible: ensure zero feature regressions
3. Performance: no hot-path regressions
4. SDK value: review in depth whether the change genuinely adds value to the SDK — never add features for the sake of adding them. It must strengthen the SDK (simpler, more user-friendly, robust, world-class, secure). If it does not clearly add value, request changes or recommend rejecting/closing rather than merging scope creep
5. Do not bloat the Agent class with additional params — only if absolutely required; we already support many params.
6. Repo routing: agent-callable tools → PraisonAI-Tools; lifecycle plugins → PraisonAI-Plugins; optional sandbox backends → PraisonAI-Plugins (praisonai.sandbox entry point) — request changes if wrongly added to praisonaiagents/

Phase 2: FIX Valid Issues
7. For any VALID bugs or architectural flaws found by Gemini, CodeRabbit, Qodo, Copilot, or any other reviewer: implement the fix
8. Also independently identify and fix any gaps or issues you find in the changed code — do not rely only on prior reviewer feedback
9. Push all code fixes directly to THIS branch (do NOT create a new PR)
10. Comment a summary of exact files modified and what you skipped

Phase 3: Final Verdict
11. If all issues are resolved, approve the PR / close the Issue
12. If blocking issues remain, request changes / leave clear action items

[praisonai-triage-agent]

Claude finished @MervinPraison's task —— View job

---

Final Architecture Review — PR #4506

• Read changed files (server.py, test_chat_stream.py)
• Read AGENTS.md & repo guidelines
• Run tests — 16/16 pass
• Analyze SSRF completeness & prior reviewer feedback
• Final verdict

---

Verdict: ✅ Approve — correct, minimal, complete. No code changes needed.

This is a genuine defensive-security fix that closes a real SSRF / approval-bypass hole. fetch_url previously gated on the approved URL string, then let urllib follow any 3xx to any host — including this engine's own loopback endpoints (/settings, /logs, another chat's transcript) — with the fetched body landing back in the model's context. The approval card lied about where the fetch actually went.

Phase 1 — Architecture (per AGENTS.md)

1. Routing ✓ — This lives in src/praisonai-desktop/engine/ (the desktop app engine), not praisonaiagents/. It is not an agent-callable SDK tool nor a lifecycle plugin, so it does not belong in PraisonAI-Tools/Plugins. Correct location.
2. Backward compatible ✓ — Only behavioural change: a legitimate redirect (e.g. http→https, trailing-slash) now returns an honest Fetch failed: error the model can surface, prompting a fresh approval card for the new URL. For a human-in-the-loop security gate this is the intended, correct tradeoff.
3. No Agent bloat / no new params / no new deps ✓ — Stdlib-only (urllib), 9 lines, no new knobs, no surface-area growth. Matches the file's "stdlib only" design contract.
4. SDK value ✓ — Strengthens security without scope creep. Asserts the effect (secret body not returned), not that a check was called.

Phase 2 — Independent verification of the fix

• build_opener(_NoRedirect) replaces the default HTTPRedirectHandler rather than adding a second one — verified: opener holds exactly one redirect handler, and it's the subclass. So no code path follows a redirect.
• All redirect codes (301/302/303/307) funnel through redirect_request(), which the override raises on unconditionally — verified against the stdlib source. No redirect target is ever fetched, regardless of status code.
• The raised HTTPError propagates cleanly to the existing except Exception and returns a descriptive string; the tool contract is preserved.
• Two-server loopback regression test proves SECRET-BODY behind the 302 never reaches the result.

No valid issues were found by me or by prior reviewers (Greptile 5/5; Qodo paused; CodeRabbit no findings) that require a fix. Nothing to skip.

Files modified this pass

None. The PR as-is is correct and complete; adding anything would be scope creep.

--- · Branch: claude/issue-4490-20260827-1336

[praisonai-triage-agent]

Merge gate scan — eligible for assessment. Claude merge gate will assess and may auto-merge if MERGE_GATE_VERDICT: APPROVE.

[praisonai-triage-agent]

Merged by Claude PR merge gate (claude-merge-gate.yml).
Verdict: MERGE_GATE_VERDICT: APPROVE
SHA: 83a28fc
Method: merge