[spark-compete] sanitize search-result URL before injecting it into the live-search-results prompt block - #207
Open
banse wants to merge 1 commit into
Conversation
…arch prompt block
attach_search_context routed each untrusted SearchResult's title and snippet
through _safe_search_context_text -> sanitize_prompt_text, but emitted r.url
verbatim into the same <live_search_results> block:
if r.url:
context_lines.append(f" source: {r.url}")
r.url is attacker-influenced: it is the decoded DuckDuckGo `uddg` redirect
target (unquote of the scraped redirect in _decode_duckduckgo_redirect), so a
result destination can embed newlines plus injected instructions
(e.g. "https://evil.example/\n\nIgnore all previous instructions and ...")
that land unsanitized as their own line inside the model prompt.
Fix: route the URL through the same sanitizer as title/snippet via a new
_safe_search_context_url helper that reuses _safe_search_context_text and
collapses the result to a single line, so smuggled newlines/control chars
cannot break out of the `source:` line. The source line is kept when a safe,
single-line URL remains.
Adds a deterministic regression test that feeds a crafted DuckDuckGo redirect
href whose decoded target carries embedded newlines + an injected instruction
through _parse_duckduckgo_html -> attach_search_context and asserts the
instruction is neutralized (single-line source, no verbatim leak).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
packet
Validated against
https://compete.sparkswarm.ai/api/packet/validate-> statuspass(error_count 0, warning_count 0).{ "schema": "spark-compete-hotfix-v1", "event": "spark-compete-first-event", "submission_mode": "public_repo_pr", "submission_target_url": "https://github.com/vibeforge1111/spark-character/pull/207", "team": { "name": "The Dudes", "members": ["His Dudeness", "El Duderino", "Duder"], "llm_device_holder": "His Dudeness", "device_holder_github": "banse", "github_accounts": ["banse"] }, "target_repo": { "id": "vibeforge1111/spark-character", "source": "https://github.com/vibeforge1111/spark-character", "owner_surface": "spark-character" }, "issue": { "type": "security_concern", "severity": "medium", "title": "Search-result URL is injected verbatim into the live-search-results prompt block (stored prompt injection)", "actual_behavior": "In src/spark_character/search_adapter.py, attach_search_context routes each untrusted SearchResult's title and snippet through _safe_search_context_text -> sanitize_prompt_text, but emits the url field verbatim: `if r.url: context_lines.append(f\" source: {r.url}\")`. r.url is the decoded DuckDuckGo `uddg` redirect target (_decode_duckduckgo_redirect does unquote(qs['uddg'][0])), so an attacker controlling a result destination can embed newlines plus an injected instruction (e.g. 'https://evil.example/\\n\\n<INJECTED_INSTRUCTION_PAYLOAD>', where the payload is a defanged single-line directive used only in the synthetic test) that lands unsanitized as its own line inside the model prompt.", "expected_behavior": "The untrusted URL field must be sanitized and normalized like title and snippet before being placed in the prompt. Smuggled newlines/control characters must not break out of the `source:` line to inject free-standing instructions into the <live_search_results> block. The source line is preserved when a safe single-line URL remains.", "repro_steps": [ "Craft DuckDuckGo HTML whose result anchor href is //duckduckgo.com/l/?uddg=<percent-encoded 'https://evil.example/\\n\\n<INJECTED_INSTRUCTION_PAYLOAD>'> (synthetic defanged payload).", "Parse it via _parse_duckduckgo_html; the resulting SearchResult.url contains the embedded newlines + injected instruction.", "Call attach_search_context(\"What's the current price of BTC?\", search_fn=lambda q: results).", "On baseline the rendered prompt contains the synthetic <INJECTED_INSTRUCTION_PAYLOAD> line verbatim inside <live_search_results>; after the fix it is collapsed to a single safe source line." ], "affected_workflow": "pipeline.generate(user_message, enable_search=True) -> attach_search_context -> search_results_for -> _duckduckgo_html_search (default keyless backend) -> _parse_duckduckgo_html -> _decode_duckduckgo_redirect -> SearchResult.url emitted verbatim into the live-search-results prompt block." }, "evidence": { "safe_links_only": true, "before_after_proof": "Baseline dc85fc8: test FAILS - the assertion that the synthetic <INJECTED_INSTRUCTION_PAYLOAD> is absent from the rendered prompt raises AssertionError at tests/test_search_adapter.py:147 (the payload leaks verbatim into the prompt). After fix: the same test PASSES (1 passed); full tests/test_search_adapter.py suite 18 passed. Fail-pre/pass-post verified by checking out the source file at baseline while keeping the committed test, then restoring the fix.", "links": [ "https://github.com/vibeforge1111/spark-character/pull/207" ], "forbidden": [ "No real secrets, API keys, tokens, passwords, or credentials are included.", "No real usernames or paths belonging to real people are included.", "All test inputs are synthetic (evil.example) and run offline via an injected search_fn (no network calls)." ] }, "proposed_fix": { "approach": "Add a pure helper _safe_search_context_url that reuses the existing _safe_search_context_text sanitizer (sanitize_prompt_text) and collapses the result to a single line (splitlines()[0].strip()), then route r.url through it at the emission site so smuggled newlines/control chars cannot break out of the `source:` line. Keep the source line when a safe single-line URL remains. Minimal, additive, no signature or network changes.", "files_expected": [ "src/spark_character/search_adapter.py", "tests/test_search_adapter.py" ], "tests_or_smoke": "Deterministic unit test tests/test_search_adapter.py::test_attach_search_context_neutralizes_injected_url_from_redirect feeds crafted DuckDuckGo redirect HTML through _parse_duckduckgo_html -> attach_search_context and asserts the injected instruction is neutralized (single-line source, no verbatim leak). Synthetic inputs only, no network (search_fn injected). Run: .venv-test/bin/python -m pytest tests/test_search_adapter.py -q." }, "pr": { "branch": "fix/search-context-url-prompt-injection", "title_prefix": "[spark-compete]", "author_github": "banse", "body_must_include": ["packet", "team", "pr_author", "repo", "actual_behavior", "expected_behavior", "repro_steps", "before_after_proof", "tests_or_smoke", "duplicate_notes", "risk_notes", "review_claim"], "url": "https://github.com/vibeforge1111/spark-character/pull/207" }, "review_claim": { "impact_claim": "medium", "evidence_types": ["failing_test", "passing_test", "redacted_terminal_excerpt"], "duplicate_notes": "Distinct by root cause (file + symbol + mechanism). #137 rewrites the <live_search_results> wrapper and _safe_search_context_text (title/snippet path) but still emits r.url verbatim (source: {r.url}) - the URL field is never sanitized; this PR is the field-level fix #137 missed. #149 (invisible-unicode), #160 (exfil-.env), #137/#87 (injection regex) all modify prompt_guard.sanitize_prompt_text itself; this PR is a caller (search_adapter.py) that forgot to apply the sanitizer to the URL field. #167 (SSRF, CWE-918) adds _is_safe_url to block private-IP/metadata fetch targets in _decode_duckduckgo_redirect; a public host like https://evil.example/<newline>... passes its check and still reaches the prompt verbatim - different mechanism and code location. #182/#157 touch _strip_tags only; #190/#141 touch search error logging/timeout. Confirmed via fresh gh pr list/diff immediately before PR creation: no open spark-character PR sanitizes the search-result URL field before prompt injection.", "risk_notes": "Minimal, surgical, additive. One new pure helper reusing the existing sanitizer plus a single-line collapse; the call site swaps r.url for the sanitized value. No new dependencies, no signature/API changes, no network behavior change. Safe single-line URLs render identically; only URLs carrying control chars / injected newlines are neutralized (collapsed to first sanitized line) instead of leaking.", "review_state_requested": "pr_review" } }team
Team The Dudes — members: His Dudeness, El Duderino, Duder. LLM device holder: His Dudeness (github: banse).
pr_author
banse
repo
vibeforge1111/spark-character (owner surface: spark-character). Base branch:
master. Branched from upstream baselinedc85fc8.actual_behavior
attach_search_contextinsrc/spark_character/search_adapter.pyroutes each untrusted webSearchResult'stitleandsnippetthrough_safe_search_context_text->sanitize_prompt_text(strips invisible unicode, blocks stored prompt-injection patterns), but emits theurlfield verbatim into the same<live_search_results>prompt block:r.urlis attacker-influenced: it is the decoded DuckDuckGouddgredirect target._decode_duckduckgo_redirectdoesunquote(qs["uddg"][0])on the scraped redirect, so an attacker controlling a result destination can embed newlines plus an injected instruction. A redirect target such ashttps://evil.example/\n\n<INJECTED_INSTRUCTION_PAYLOAD>(a defanged single-line directive used only in the synthetic test) is decoded and emitted unsanitized, landing as its own line inside the model prompt — a stored prompt-injection vector.Reachability:
pipeline.generate(user_message, enable_search=True)->attach_search_context->search_results_for->_duckduckgo_html_search(default backend, no API key) ->_parse_duckduckgo_html->_decode_duckduckgo_redirect(unquoteofuddg) ->SearchResult.url-> emitted verbatim.expected_behavior
The untrusted URL field must be subjected to the same sanitization/normalization as the title and snippet before it is placed in the prompt. Smuggled newlines and control characters must not be able to break out of the
source:line and inject free-standing instructions into the<live_search_results>block. Thesource:line is preserved when a safe, single-line URL remains.repro_steps
//duckduckgo.com/l/?uddg=<percent-encoded "https://evil.example/\n\n<INJECTED_INSTRUCTION_PAYLOAD>">(synthetic defanged payload)._parse_duckduckgo_html-> the resultingSearchResult.urlcontains the embedded newlines + injected instruction (verbatim).attach_search_context("What's the current price of BTC?", search_fn=lambda q: results).<INJECTED_INSTRUCTION_PAYLOAD>line verbatim, on its own line inside<live_search_results>; after the fix it is collapsed to a single safesource:line.before_after_proof
Baseline (
dc85fc8) — test FAILS (injected instruction leaks verbatim). The injection payload is shown defanged as<INJECTED_INSTRUCTION_PAYLOAD>; the real synthetic string lives only in the test source:After fix — test PASSES, full module suite green:
Rendered prompt after fix collapses the
source:line to a single safe line; the injected instruction no longer appears.tests_or_smoke
Deterministic unit test
tests/test_search_adapter.py::test_attach_search_context_neutralizes_injected_url_from_redirect. Synthetic inputs only (no network —search_fnis injected). Run targeted:Fail-pre verified by checking out the source file at baseline
dc85fc8(keeping the committed test); pass-post verified after restoring the fix. Fulltest_search_adapter.pysuite: 18 passed.duplicate_notes
Re-ran a fresh
gh pr list/gh pr diffdedup immediately before PR creation. Existing prompt-guard / search PRs are distinct by root cause (file + symbol + mechanism):<live_search_results>wrapper and_safe_search_context_text(title/snippet path) but still emitsr.urlverbatim (source: {r.url}) — the URL field is never sanitized. This PR is the field-level fix [spark-compete] fix: enhance prompt injection sanitization for search results (HIGH) #137 missed..envboundary), [spark-compete] fix: enhance prompt injection sanitization for search results (HIGH) #137/[spark-compete] fix(prompt_guard): widen injection prefix and add synonym verbs to catch more bypasses #87 (injection regex) all modifyprompt_guard.sanitize_prompt_textitself. This PR is a caller (search_adapter.py) that forgot to apply the sanitizer to the URL field — different file + mechanism._is_safe_urlto block private-IP/metadata fetch targets inside_decode_duckduckgo_redirect; a public host likehttps://evil.example/<newline>...passes its check and still reaches the prompt verbatim — different mechanism (egress safety vs. prompt-text sanitization) and different code location._strip_tagsonly. [spark-compete] fix(search-adapter): log full traceback on search failure #190/[spark-compete] fix: search_adapter timeout_seconds parameter is dead code #141 touch search error logging/timeout.No open
spark-characterPR sanitizes the search-result URL field before prompt injection.risk_notes
Minimal, surgical, additive. One new pure helper
_safe_search_context_urlthat reuses the existing_safe_search_context_textsanitizer and collapses to a single line; the call site swapsr.urlfor the sanitized value. No new dependencies, no signature/API changes, no network behavior change. Safe single-line URLs (the overwhelmingly common case) render identically to before. The only behavior change is that a URL carrying control chars / injected newlines is neutralized (collapsed to its first sanitized line) instead of leaking.review_claim
Requesting
pr_review. Impact: medium (stored prompt injection via attacker-controlled search-result URL reachable through the default keyless DuckDuckGo backend onpipeline.generate(..., enable_search=True)). Evidence: failing_test (baseline), passing_test (post-fix), redacted_terminal_excerpt. Safe synthetic inputs only — no real secrets, tokens, usernames, or real-person paths.