FIRE-1166 | Red-Team | HTML Indirect prompt injection attack#160
Open
FIRE-1166 | Red-Team | HTML Indirect prompt injection attack#160
Conversation
Contributor
Summary by CodeRabbit
WalkthroughAdds a new premium attack "html-indirect-prompt-injection", threads an optional Changes
Sequence Diagram(s)sequenceDiagram
participant TUI as TUI (Client)
participant Orch as RedTeamOrchestrator
participant Factory as AttackerFactory
participant Agent as RedTeamAttackerAgent
participant Deckard as Deckard Service
TUI->>Orch: Start red-team job (includes deckard_base_url, auth)
Orch->>Factory: create_red_team_attacker_agent(..., deckard_base_url)
Factory->>Agent: instantiate agent(..., deckard_base_url)
Agent->>Deckard: call premium endpoints (uses deckard_base_url + X-Qualifire-API-Key)
Deckard-->>Agent: attack payloads / responses
Agent-->>Orch: results
Orch-->>TUI: job status/results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rogue/server/qualifire_attacks.py (1)
76-83:⚠️ Potential issue | 🟡 MinorVerify header naming consistency between DeckardClient and Qualifire service integration.
The DeckardClient uses
X-Qualifire-API-Keywhile the established Qualifire service integration usesX-qualifire-key(line 30 & 148 ofrogue/server/services/qualifire_service.py). Both default to the same backend URL (https://app.qualifire.ai), which suggests they target the same service. Before rollout, confirm whether the backend accepts the new header name or if both variants need to be supported for backward compatibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rogue/server/qualifire_attacks.py` around lines 76 - 83, The header name used in _get_headers (X-Qualifire-API-Key) is inconsistent with the header used in the Qualifire integration (X-qualifire-key) and may cause auth failures; update _get_headers in the DeckardClient (or the class containing that method) to use the canonical header name used by the Qualifire service (X-qualifire-key), and/or add both headers when self.api_key is present to preserve backward compatibility (i.e., set both "X-qualifire-key" and "X-Qualifire-API-Key" to self.api_key), ensuring the change is applied where _get_headers is invoked so requests to https://app.qualifire.ai authenticate correctly.
🧹 Nitpick comments (1)
rogue/evaluator_agent/red_team/factory.py (1)
68-68: Consider documenting the new parameter in the docstring.The
deckard_base_urlparameter is not documented in the function's docstring (lines 73-96). While this follows the existing pattern (some parameters likeattacker_llm_api_baseare also undocumented), consider adding documentation for clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rogue/evaluator_agent/red_team/factory.py` at line 68, The new deckard_base_url parameter is missing from the function's docstring; update the docstring for the factory function that defines deckard_base_url to include a concise description (purpose, type Optional[str], and expected format/example) following the same style as other parameters (e.g., attacker_llm_api_base) so readers can understand its use and expected value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@rogue/server/qualifire_attacks.py`:
- Around line 76-83: The header name used in _get_headers (X-Qualifire-API-Key)
is inconsistent with the header used in the Qualifire integration
(X-qualifire-key) and may cause auth failures; update _get_headers in the
DeckardClient (or the class containing that method) to use the canonical header
name used by the Qualifire service (X-qualifire-key), and/or add both headers
when self.api_key is present to preserve backward compatibility (i.e., set both
"X-qualifire-key" and "X-Qualifire-API-Key" to self.api_key), ensuring the
change is applied where _get_headers is invoked so requests to
https://app.qualifire.ai authenticate correctly.
---
Nitpick comments:
In `@rogue/evaluator_agent/red_team/factory.py`:
- Line 68: The new deckard_base_url parameter is missing from the function's
docstring; update the docstring for the factory function that defines
deckard_base_url to include a concise description (purpose, type Optional[str],
and expected format/example) following the same style as other parameters (e.g.,
attacker_llm_api_base) so readers can understand its use and expected value.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/tui/internal/screens/evaluations/form_view.go (1)
300-309: Consider using FormField constants instead of hard-coded indices.The Python protocol path uses hard-coded indices (5, 6, 7) for Judge LLM, Deep Test, and Mode fields. Consider using
int(FormFieldJudgeModel),int(FormFieldDeepTest), andint(FormFieldEvaluationMode)for maintainability.♻️ Suggested improvement
formFields = []string{ renderDropdownField(0, "Protocol:", protocol), renderTextField(1, "Python File:", pythonFile), // No Transport, AuthType, AuthCredentials for Python protocol - renderTextField(5, "Judge LLM:", judge), - renderToggleField(6, "Deep Test:", deep), - renderDropdownField(7, "Mode:", evalMode), + renderTextField(int(FormFieldJudgeModel), "Judge LLM:", judge), + renderToggleField(int(FormFieldDeepTest), "Deep Test:", deep), + renderDropdownField(int(FormFieldEvaluationMode), "Mode:", evalMode), }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tui/internal/screens/evaluations/form_view.go` around lines 300 - 309, The Python protocol form uses hard-coded field indices for Judge LLM, Deep Test and Mode which risks drift; replace the numeric indices (5,6,7) in the python branch where formFields is built (the calls to renderTextField, renderToggleField, renderDropdownField) with the corresponding FormField constants cast to int (int(FormFieldJudgeModel), int(FormFieldDeepTest), int(FormFieldEvaluationMode)) so the fields align with the rest of the form and remain maintainable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/tui/internal/tui/utils.go`:
- Line 40: The struct field EvaluatedAgentCredentials in
packages/tui/internal/tui/utils.go was renamed in its JSON tag to
evaluated_agent_auth_credentials which breaks the backend API contract; change
the struct tag back to `json:"evaluated_agent_credentials,omitempty"` so the TUI
sends the expected field name (leave the Go field name EvaluatedAgentCredentials
as-is, only update the JSON tag).
---
Nitpick comments:
In `@packages/tui/internal/screens/evaluations/form_view.go`:
- Around line 300-309: The Python protocol form uses hard-coded field indices
for Judge LLM, Deep Test and Mode which risks drift; replace the numeric indices
(5,6,7) in the python branch where formFields is built (the calls to
renderTextField, renderToggleField, renderDropdownField) with the corresponding
FormField constants cast to int (int(FormFieldJudgeModel),
int(FormFieldDeepTest), int(FormFieldEvaluationMode)) so the fields align with
the rest of the form and remain maintainable.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/tui/internal/tui/form_controller.go (2)
99-106: Combine duplicate case branches for cleaner code.The
EvalFieldAuthCredentialscase performs the same cursor decrement as the combinedEvalFieldAgentURL, EvalFieldJudgeModelcase.♻️ Proposed consolidation
- case EvalFieldAgentURL, EvalFieldJudgeModel: // Text fields - move cursor left - if m.evalState.cursorPos > 0 { - m.evalState.cursorPos-- - } - case EvalFieldAuthCredentials: // Text field - move cursor left + case EvalFieldAgentURL, EvalFieldJudgeModel, EvalFieldAuthCredentials: // Text fields - move cursor left if m.evalState.cursorPos > 0 { m.evalState.cursorPos-- }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tui/internal/tui/form_controller.go` around lines 99 - 106, The switch contains duplicate branches that decrement m.evalState.cursorPos for EvalFieldAgentURL, EvalFieldJudgeModel and EvalFieldAuthCredentials; consolidate them into a single case list (e.g. case EvalFieldAgentURL, EvalFieldJudgeModel, EvalFieldAuthCredentials:) and keep the shared body that checks and decrements m.evalState.cursorPos to remove the duplicated logic.
79-92: Consider extracting cursor position helper to reduce duplication.The cursor position setting logic (lines 79-92) is identical to lines 38-51. Extracting this into a helper method would improve maintainability.
♻️ Proposed helper extraction
+// setCursorToEndOfField sets cursor position to end of the current field's content +func (s *EvaluationViewState) setCursorToEndOfField() { + switch s.currentField { + case EvalFieldAgentURL: + if s.AgentProtocol == ProtocolPython { + s.cursorPos = len([]rune(s.PythonEntrypointFile)) + } else { + s.cursorPos = len([]rune(s.AgentURL)) + } + case EvalFieldAuthCredentials: + s.cursorPos = len([]rune(s.AgentAuthCredentials)) + case EvalFieldJudgeModel: + s.cursorPos = len([]rune(s.JudgeModel)) + default: + s.cursorPos = 0 + } +}Then replace both occurrences with:
m.evalState.setCursorToEndOfField()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tui/internal/tui/form_controller.go` around lines 79 - 92, The cursor-position-setting switch in form_controller.go is duplicated; extract it into a helper on the evalState struct (e.g., func (s *EvalState) setCursorToEndOfField()) that reads s.currentField, s.AgentProtocol, s.PythonEntrypointFile, s.AgentURL, s.AgentAuthCredentials and s.JudgeModel and sets s.cursorPos appropriately (use len([]rune(...)) for each string and 0 default). Replace both occurrences with a call to m.evalState.setCursorToEndOfField().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/tui/internal/tui/form_controller.go`:
- Around line 99-106: The switch contains duplicate branches that decrement
m.evalState.cursorPos for EvalFieldAgentURL, EvalFieldJudgeModel and
EvalFieldAuthCredentials; consolidate them into a single case list (e.g. case
EvalFieldAgentURL, EvalFieldJudgeModel, EvalFieldAuthCredentials:) and keep the
shared body that checks and decrements m.evalState.cursorPos to remove the
duplicated logic.
- Around line 79-92: The cursor-position-setting switch in form_controller.go is
duplicated; extract it into a helper on the evalState struct (e.g., func (s
*EvalState) setCursorToEndOfField()) that reads s.currentField, s.AgentProtocol,
s.PythonEntrypointFile, s.AgentURL, s.AgentAuthCredentials and s.JudgeModel and
sets s.cursorPos appropriately (use len([]rune(...)) for each string and 0
default). Replace both occurrences with a call to
m.evalState.setCursorToEndOfField().
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
1 participant
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.
Description
Motivation and Context
Type of Change
Changes Made
Screenshots/Examples (if applicable)
Checklist
uv run black .to format my codeuv run flake8 .and fixed all issuesuv run mypy --config-file .mypy.ini .and addressed type checking issuesuv run bandit -c .bandit.yaml -r .for security checksuv run pytestand all tests passTesting
Test Configuration:
Test Steps:
1.
2.
3.
Additional Notes
Related Issues/PRs