-
Notifications
You must be signed in to change notification settings - Fork 146
feature/red team report FIRE 1004 #155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Summary by CodeRabbit
WalkthroughThis PR adds comprehensive Rogue documentation, a root Makefile and dev tooling, tmux/healthcheck scripts, port pre-flight checks, and new Qualifire red-team reporting end-to-end (server endpoint, service, SDK/TUI plumbing, response fields, and UI link rendering). It also removes the package-level TUI Makefile. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TUI as TUI Client
participant Server as Rogue Server
participant Qualifire as Qualifire API
User->>TUI: Save/Report red-team results
TUI->>Server: POST /red-team/{job_id}/report-to-qualifire<br/>(qualifire_api_key)
Server->>Server: Build red-team payload (job, vulnerabilities, attacks, frameworks)
Server->>Qualifire: POST /api/rogue/v1/red-team (payload + api_key)
Qualifire-->>Server: {scan_id, report_id}
Server-->>TUI: Respond with summary + report_url
TUI->>TUI: Extract scan_id from report_url, store redTeamScanID
TUI->>TUI: Prepend styled Qualifire link to rendered report
TUI-->>User: Display clickable Qualifire report link
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 14
🤖 Fix all issues with AI agents
In `@docs/rogue/cli.mdx`:
- Around line 12-26: The "default mode" bullet in the CLI docs incorrectly
implies the CLI will auto-start the server; update the text under the CLI
section to remove or reword that bullet so it explicitly states the "default
mode" refers to the TUI only (not CLI). Locate the phrase "Use the default mode
(starts server + TUI, then use TUI for evaluation)" in docs/rogue/cli.mdx and
either delete it or replace it with a clear note such as "The default mode
launches the TUI (not the CLI); to use CLI, start the server separately" so
readers won't be misled.
In `@docs/rogue/concepts/frameworks.mdx`:
- Around line 92-100: Update the API7 table label to use the hyphenated compound
adjective: change the text "API7: Server Side Request Forgery" to "API7:
Server-Side Request Forgery" in the frameworks.mdx table row (the row mapping
Category "API7" to mapped vulnerability "ssrf"); preserve the rest of the cell
content and casing and ensure the Markdown table alignment remains intact.
In `@docs/rogue/protocols/a2a.mdx`:
- Around line 105-141: The example references undefined variables host and port
used when constructing agent_card.url and calling uvicorn.run; define
placeholder values or configuration variables for host and port (e.g., host =
"127.0.0.1" and port = 8000) before they are used so agent_card, uvicorn.run,
and any URL formatting have valid values; alternatively add a clear inline
comment above agent_card and uvicorn.run explaining these must be provided from
configuration or environment.
In `@docs/rogue/protocols/mcp.mdx`:
- Line 3: The front-matter description string contains a typo: change the
description value that currently reads "Using MCO (Model Context Protocol) with
Rogue" to "Using MCP (Model Context Protocol) with Rogue" so the abbreviation
matches the documented protocol; update the description field in the document's
metadata accordingly.
In `@docs/rogue/protocols/python.mdx`:
- Around line 192-220: The example references undefined symbols
create_initial_state and graph which will confuse readers; add small stub
definitions or a clarifying comment above call_agent indicating these are
placeholders. Specifically, define or document a create_initial_state function
(used to initialize state) and a graph object with an invoke method (used in
graph.invoke(...)) so the reader can run or replace them, or replace the calls
with commented placeholders referencing these symbols.
In `@docs/rogue/supported-models.mdx`:
- Around line 24-44: The gemini-2.5-flash entry is listed both under "Gemini
(Vertex AI or Google AI)" as a successfully tested model and under "Unsupported
Models" as partially supported; resolve the conflict by choosing one status:
either remove "gemini-2.5-flash" from the successfully tested list or from the
unsupported list, or add a new "Partially Supported" section and place
"gemini-2.5-flash" there; update only the model listing lines that mention
gemini-2.5-flash so the document shows a single, consistent status.
In `@packages/tui/internal/tui/common_controller.go`:
- Around line 212-219: The current extraction from msg.ReportURL (in the block
setting m.redTeamScanID) can produce an empty string for trailing slashes or
malformed URLs; update the logic to robustly parse the URL path (e.g., use
net/url to parse msg.ReportURL, take url.Path or path.Base, then strings.Trim to
remove trailing slashes) and only assign to m.redTeamScanID if the resulting
segment is non-empty (otherwise leave it unchanged or handle as an error);
reference the existing symbols msg.ReportURL and m.redTeamScanID when making
this change.
- Around line 410-446: The SDK error dialog set after ReportRedTeamScan or
ReportSummary can be overwritten later by the success dialog because execution
continues to call config.Save and show success; modify the control flow in the
function containing this block so that when either sdk.ReportRedTeamScan or
sdk.ReportSummary returns a non-nil err you set m.dialog (as already done) and
then return early (or otherwise skip the rest of the success path) to avoid
calling config.Save and the subsequent success dialog; specifically update the
handling around ReportRedTeamScan and ReportSummary to return immediately after
setting m.dialog (or propagate the error) so the success dialog logic cannot
overwrite the error dialog.
- Around line 238-254: The goroutine is causing a data race by writing
m.redTeamScanID and calling m.initializeRedTeamReportViewport() directly;
instead implement a Bubble Tea command that performs the network call and
returns a message which the Update loop handles. Add a message type (e.g.
QualifireReportResultMsg with ScanID string and Err error), replace the inline
goroutine with a tea.Cmd that calls NewRogueSDK(...).ReportRedTeamScan and
returns QualifireReportResultMsg, and in the model's Update switch on
QualifireReportResultMsg to set m.redTeamScanID and call
m.initializeRedTeamReportViewport() (only on success) so all state mutations
happen inside the Bubble Tea update loop.
In `@packages/tui/internal/tui/redteam_report_utils.go`:
- Around line 29-37: theme.CurrentTheme() may return nil so calling t.Success()
/ t.Background() can panic; guard the call in the block that builds linkStyle by
checking if theme.CurrentTheme() == nil and assign a safe fallback (e.g.
theme.DefaultTheme() or a constructed default Theme) to t before using it, then
proceed to build lipgloss.NewStyle() into linkStyle as before (refer to
theme.CurrentTheme(), t.Success(), t.Background(), and linkStyle).
In `@rogue/run_server.py`:
- Around line 140-148: The port-check currently binds using socket.AF_INET which
fails for IPv6 hosts; update the pre-start check (where host and port are used
and the socket.connect_ex call is made) to resolve the host with
socket.getaddrinfo and attempt a connect_ex for each returned addrinfo using its
provided family (AF_INET or AF_INET6) and sockaddr, raising a more specific
exception type like RuntimeError if any connect succeeds; ensure the error
message keeps the same guidance about stopping the other process or changing
PORT.
In `@rogue/server/main.py`:
- Around line 94-105: Replace the connect_ex-based port check with a bind
attempt inside a try/except: create the same AF_INET/SOCK_STREAM socket (using
the existing with socket.socket(...) as sock), call
sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) if desired, then try
sock.bind((host, port)); on success the port is available (socket will be closed
by the with block), on failure catch OSError/Exception, log the error with
logger.error including the exception details and the existing
extra={"component":"server_main","host":host,"port":port}, and call sys.exit(1).
Update the code that currently calls sock.connect_ex((host, port)) to this
bind/try/except pattern so the check verifies bindability per guidelines.
In `@rogue/server/services/qualifire_service.py`:
- Around line 127-142: The current mapping in the list comprehension for
report.vulnerability_table sets "cvss_score": vt.success_rate which is incorrect
because VulnerabilityTableRow has no cvss_score; either remove the "cvss_score"
key from the dict produced in that comprehension (the one building "breakdown")
or, if the API requires it, pull the real CVSS value from the underlying
VulnerabilityResult (e.g., access the CVSS field on the associated
vulnerability/result object) and assign that instead (replace vt.success_rate
with the correct attribute like vulnerability_result.cvss_score or similar);
update the list comprehension that iterates report.vulnerability_table
accordingly and ensure any callers expecting cvss_score are adjusted.
In `@scripts/healthcheck.sh`:
- Around line 23-31: The status line claims the script "refreshes every 5s" but
there is no loop or sleep; wrap the display and the three check_service calls
(the block containing the printf with "Rogue Services", the check_service calls
for "Server", "TShirt Agent", "Langgraph Agent", and the final printf showing
refresh text) inside a while true loop that clears the screen, runs those
checks, then sleeps 5, or alternatively change the final printf text to remove
the "refreshes every 5s" claim; update healthcheck.sh accordingly so the message
matches behavior.
🧹 Nitpick comments (6)
scripts/dev-tmux.sh (1)
53-69: Derive pane titles fromSERVICESto avoid duplication.
You already parsenamein the loop; use it to set the pane title and remove the hard-coded title block.♻️ Proposed refactor
for i in "${!SERVICES[@]}"; do IFS='|' read -r name cmd <<< "${SERVICES[$i]}" pane_idx=${PANE_ORDER[$i]} tmux send-keys -t "$SESSION_NAME:0.$pane_idx" "$ENV_CMD && $cmd" C-m + tmux select-pane -t "$SESSION_NAME:0.$pane_idx" -T "$name" done @@ -tmux select-pane -t "$SESSION_NAME:0.0" -T "Server" -tmux select-pane -t "$SESSION_NAME:0.1" -T "TUI" -tmux select-pane -t "$SESSION_NAME:0.2" -T "TShirt Agent" -tmux select-pane -t "$SESSION_NAME:0.3" -T "Langgraph Agent"Makefile (1)
3-3: Expand.PHONYto cover all declared targets.
Prevents accidental conflicts if files with the same names appear.♻️ Proposed fix
-.PHONY: build clean run test install deps dev dev-tmux stop-tmux +.PHONY: all build build-all clean run test test-coverage install deps dev fmt lint vet security check setup dev-tmux stop-tmux helppackages/tui/internal/tui/redteam_report_utils.go (1)
44-46: Minor:strings.Joinunnecessary for two elements.Using
strings.Joinfor concatenating exactly two strings adds overhead. A simple concatenation orfmt.Sprintfwould be cleaner.♻️ Simpler concatenation
- linkText := linkStyle.Render( - strings.Join([]string{"✅ Report saved to Qualifire: ", reportURL}, ""), - ) + linkText := linkStyle.Render("✅ Report saved to Qualifire: " + reportURL)docs/rogue/architecture.mdx (1)
137-154: Minor: ASCII diagram box widths are inconsistent.The outer box header line (line 138) and content lines have slightly different character counts, causing visual misalignment in some renderers. This is cosmetic only.
rogue/server/services/qualifire_service.py (1)
44-50: Add type hints forjobandreportparameters.The function signature is missing type hints for
jobandreport, which is required per coding guidelines. The docstring describes the types but actual annotations are absent.♻️ Proposed fix to add type hints
+from rogue_sdk.types import RedTeamJob +from ..red_teaming.report.types import RedTeamReport + `@staticmethod` def report_red_team_scan( - job, - report, + job: RedTeamJob, + report: "RedTeamReport", qualifire_api_key: str, qualifire_url: str = "", ):Note: The exact import paths may need adjustment based on where
RedTeamReportis defined.As per coding guidelines: "Use type hints for all function signatures"
rogue/server/api/red_team.py (1)
292-303: Consider extracting report generation to a helper method.The
ComplianceReportGeneratorinstantiation andgenerate_full_reportcall pattern is repeated in three places:
generate_red_team_summary(lines 203-210, 292-303)report_red_team_to_qualifire(lines 385-404)get_red_team_report(lines 472-494)This is acceptable for now but could be consolidated into a helper if the file grows further.
Also applies to: 383-404, 468-494
| The CLI mode requires the Rogue server to be running. You can either: | ||
|
|
||
| 1. **Start server separately:** | ||
|
|
||
| ```bash | ||
| # Terminal 1: Start the server | ||
| uvx rogue-ai server | ||
|
|
||
| # Terminal 2: Run CLI evaluation | ||
| uvx rogue-ai cli [OPTIONS] | ||
| ``` | ||
|
|
||
| 2. **Use the default mode (starts server + TUI, then use TUI for evaluation)** | ||
|
|
||
| For development or if you prefer to install locally: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clarify the “default mode” bullet—this is TUI, not CLI. The CLI section currently describes starting server + TUI, which can mislead readers into thinking CLI can auto-start the server. Consider removing the bullet or explicitly marking it as TUI-only.
📝 Suggested edit (mark as TUI-only)
-2. **Use the default mode (starts server + TUI, then use TUI for evaluation)**
+2. **(TUI only)** If you want Rogue to start the server automatically, use the TUI flow (not CLI).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| The CLI mode requires the Rogue server to be running. You can either: | |
| 1. **Start server separately:** | |
| ```bash | |
| # Terminal 1: Start the server | |
| uvx rogue-ai server | |
| # Terminal 2: Run CLI evaluation | |
| uvx rogue-ai cli [OPTIONS] | |
| ``` | |
| 2. **Use the default mode (starts server + TUI, then use TUI for evaluation)** | |
| For development or if you prefer to install locally: | |
| The CLI mode requires the Rogue server to be running. You can either: | |
| 1. **Start server separately:** |
🤖 Prompt for AI Agents
In `@docs/rogue/cli.mdx` around lines 12 - 26, The "default mode" bullet in the
CLI docs incorrectly implies the CLI will auto-start the server; update the text
under the CLI section to remove or reword that bullet so it explicitly states
the "default mode" refers to the TUI only (not CLI). Locate the phrase "Use the
default mode (starts server + TUI, then use TUI for evaluation)" in
docs/rogue/cli.mdx and either delete it or replace it with a clear note such as
"The default mode launches the TUI (not the CLI); to use CLI, start the server
separately" so readers won't be misled.
| | Category | Mapped Vulnerabilities | | ||
| |----------|----------------------| | ||
| | **API1: Broken Object Level Authorization** | bola | | ||
| | **API2: Broken Authentication** | rbac | | ||
| | **API3: Broken Object Property Level Authorization** | bfla | | ||
| | **API4: Unrestricted Resource Consumption** | unbounded-consumption | | ||
| | **API5: Broken Function Level Authorization** | excessive-agency | | ||
| | **API7: Server Side Request Forgery** | ssrf | | ||
| | **API8: Security Misconfiguration** | debug-access | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hyphenate “Server-Side” in API7 label.
Minor readability polish for the compound adjective.
✏️ Proposed fix
-| **API7: Server Side Request Forgery** | ssrf |
+| **API7: Server-Side Request Forgery** | ssrf |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | Category | Mapped Vulnerabilities | | |
| |----------|----------------------| | |
| | **API1: Broken Object Level Authorization** | bola | | |
| | **API2: Broken Authentication** | rbac | | |
| | **API3: Broken Object Property Level Authorization** | bfla | | |
| | **API4: Unrestricted Resource Consumption** | unbounded-consumption | | |
| | **API5: Broken Function Level Authorization** | excessive-agency | | |
| | **API7: Server Side Request Forgery** | ssrf | | |
| | **API8: Security Misconfiguration** | debug-access | | |
| | Category | Mapped Vulnerabilities | | |
| |----------|----------------------| | |
| | **API1: Broken Object Level Authorization** | bola | | |
| | **API2: Broken Authentication** | rbac | | |
| | **API3: Broken Object Property Level Authorization** | bfla | | |
| | **API4: Unrestricted Resource Consumption** | unbounded-consumption | | |
| | **API5: Broken Function Level Authorization** | excessive-agency | | |
| | **API7: Server-Side Request Forgery** | ssrf | | |
| | **API8: Security Misconfiguration** | debug-access | |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~99-~99: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...zation** | excessive-agency | | API7: Server Side Request Forgery | ssrf | | **API8: Se...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🤖 Prompt for AI Agents
In `@docs/rogue/concepts/frameworks.mdx` around lines 92 - 100, Update the API7
table label to use the hyphenated compound adjective: change the text "API7:
Server Side Request Forgery" to "API7: Server-Side Request Forgery" in the
frameworks.mdx table row (the row mapping Category "API7" to mapped
vulnerability "ssrf"); preserve the rest of the cell content and casing and
ensure the Markdown table alignment remains intact.
| agent_card = AgentCard( | ||
| name="My Agent", | ||
| description="My Agent Description", | ||
| url=f"http://{host}:{port}/", | ||
| version="1.0.0", | ||
| defaultInputModes=["text"], | ||
| defaultOutputModes=["text"], | ||
| capabilities=AgentCapabilities(), | ||
| skills=[AgentSkill(...), AgentSkill(...)], | ||
| ) | ||
|
|
||
| agent = get_agent() | ||
|
|
||
| runner = Runner( | ||
| app_name=agent_card.name, | ||
| agent=agent, | ||
| artifact_service=InMemoryArtifactService(), | ||
| session_service=InMemorySessionService(), | ||
| memory_service=InMemoryMemoryService(), | ||
| ) | ||
| agent_executor = MyAgentExecutor(runner, agent_card) | ||
|
|
||
| request_handler = DefaultRequestHandler( | ||
| agent_executor=agent_executor, | ||
| task_store=InMemoryTaskStore(), | ||
| ) | ||
|
|
||
| a2a_app = A2AStarletteApplication( | ||
| agent_card=agent_card, | ||
| http_handler=request_handler, | ||
| ) | ||
|
|
||
| uvicorn.run( | ||
| a2a_app.build(), | ||
| host=host, | ||
| port=port, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code example uses undefined variables host and port.
The example references host and port variables (lines 108, 139, 140) that are not defined. Consider adding placeholder definitions or a comment indicating these should be configured.
📝 Suggested addition
+# Configuration
+host = "0.0.0.0"
+port = 10001
+
agent_card = AgentCard(
name="My Agent",
description="My Agent Description",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| agent_card = AgentCard( | |
| name="My Agent", | |
| description="My Agent Description", | |
| url=f"http://{host}:{port}/", | |
| version="1.0.0", | |
| defaultInputModes=["text"], | |
| defaultOutputModes=["text"], | |
| capabilities=AgentCapabilities(), | |
| skills=[AgentSkill(...), AgentSkill(...)], | |
| ) | |
| agent = get_agent() | |
| runner = Runner( | |
| app_name=agent_card.name, | |
| agent=agent, | |
| artifact_service=InMemoryArtifactService(), | |
| session_service=InMemorySessionService(), | |
| memory_service=InMemoryMemoryService(), | |
| ) | |
| agent_executor = MyAgentExecutor(runner, agent_card) | |
| request_handler = DefaultRequestHandler( | |
| agent_executor=agent_executor, | |
| task_store=InMemoryTaskStore(), | |
| ) | |
| a2a_app = A2AStarletteApplication( | |
| agent_card=agent_card, | |
| http_handler=request_handler, | |
| ) | |
| uvicorn.run( | |
| a2a_app.build(), | |
| host=host, | |
| port=port, | |
| ) | |
| # Configuration | |
| host = "0.0.0.0" | |
| port = 10001 | |
| agent_card = AgentCard( | |
| name="My Agent", | |
| description="My Agent Description", | |
| url=f"http://{host}:{port}/", | |
| version="1.0.0", | |
| defaultInputModes=["text"], | |
| defaultOutputModes=["text"], | |
| capabilities=AgentCapabilities(), | |
| skills=[AgentSkill(...), AgentSkill(...)], | |
| ) | |
| agent = get_agent() | |
| runner = Runner( | |
| app_name=agent_card.name, | |
| agent=agent, | |
| artifact_service=InMemoryArtifactService(), | |
| session_service=InMemorySessionService(), | |
| memory_service=InMemoryMemoryService(), | |
| ) | |
| agent_executor = MyAgentExecutor(runner, agent_card) | |
| request_handler = DefaultRequestHandler( | |
| agent_executor=agent_executor, | |
| task_store=InMemoryTaskStore(), | |
| ) | |
| a2a_app = A2AStarletteApplication( | |
| agent_card=agent_card, | |
| http_handler=request_handler, | |
| ) | |
| uvicorn.run( | |
| a2a_app.build(), | |
| host=host, | |
| port=port, | |
| ) |
🤖 Prompt for AI Agents
In `@docs/rogue/protocols/a2a.mdx` around lines 105 - 141, The example references
undefined variables host and port used when constructing agent_card.url and
calling uvicorn.run; define placeholder values or configuration variables for
host and port (e.g., host = "127.0.0.1" and port = 8000) before they are used so
agent_card, uvicorn.run, and any URL formatting have valid values; alternatively
add a clear inline comment above agent_card and uvicorn.run explaining these
must be provided from configuration or environment.
| ```python | ||
| from langgraph.graph import StateGraph | ||
| from typing import Any, Optional | ||
|
|
||
| # Session storage for stateful agents | ||
| sessions: dict[str, Any] = {} | ||
|
|
||
| def call_agent( | ||
| messages: list[dict[str, Any]], | ||
| context_id: Optional[str] = None, | ||
| ) -> str: | ||
| # Get or create session state | ||
| if context_id and context_id in sessions: | ||
| state = sessions[context_id] | ||
| else: | ||
| state = create_initial_state() | ||
|
|
||
| # Run your graph | ||
| result = graph.invoke( | ||
| {"messages": messages}, | ||
| config={"configurable": {"thread_id": context_id}} | ||
| ) | ||
|
|
||
| # Store session state | ||
| if context_id: | ||
| sessions[context_id] = result | ||
|
|
||
| return result["messages"][-1].content | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code example references undefined functions.
The LangGraph example calls create_initial_state() (line 207) and uses graph (line 210) without defining them. Consider adding stub definitions or a comment clarifying these are placeholders.
🤖 Prompt for AI Agents
In `@docs/rogue/protocols/python.mdx` around lines 192 - 220, The example
references undefined symbols create_initial_state and graph which will confuse
readers; add small stub definitions or a clarifying comment above call_agent
indicating these are placeholders. Specifically, define or document a
create_initial_state function (used to initialize state) and a graph object with
an invoke method (used in graph.invoke(...)) so the reader can run or replace
them, or replace the calls with commented placeholders referencing these
symbols.
| if m.redTeamScanID != "" { | ||
| t := theme.CurrentTheme() | ||
| linkStyle := lipgloss.NewStyle(). | ||
| Foreground(t.Success()). | ||
| Background(t.Background()). | ||
| Bold(true). | ||
| Width(m.width). | ||
| Align(lipgloss.Center). | ||
| Padding(1, 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential nil pointer dereference on theme.CurrentTheme().
Per the theme/manager.go implementation, CurrentTheme() returns nil when no theme is set. Calling methods like t.Success() or t.Background() on a nil Theme will cause a panic.
🐛 Proposed fix to add nil guard
// Prepend Qualifire link if available
if m.redTeamScanID != "" {
t := theme.CurrentTheme()
+ if t == nil {
+ // Fallback: skip styling if no theme available
+ qualifireBase := os.Getenv("QUALIFIRE_URL")
+ if qualifireBase == "" {
+ qualifireBase = "https://app.qualifire.ai"
+ }
+ reportURL := fmt.Sprintf("%s/red-team/%s", qualifireBase, m.redTeamScanID)
+ content = fmt.Sprintf("✅ Report saved to Qualifire: %s\n\n%s", reportURL, content)
+ } else {
linkStyle := lipgloss.NewStyle().
Foreground(t.Success()).
Background(t.Background()).
Bold(true).
Width(m.width).
Align(lipgloss.Center).
Padding(1, 0)
+ // ... rest of styled rendering
+ }Alternatively, ensure CurrentTheme() always returns a valid theme or use a default theme.
🤖 Prompt for AI Agents
In `@packages/tui/internal/tui/redteam_report_utils.go` around lines 29 - 37,
theme.CurrentTheme() may return nil so calling t.Success() / t.Background() can
panic; guard the call in the block that builds linkStyle by checking if
theme.CurrentTheme() == nil and assign a safe fallback (e.g.
theme.DefaultTheme() or a constructed default Theme) to t before using it, then
proceed to build lipgloss.NewStyle() into linkStyle as before (refer to
theme.CurrentTheme(), t.Success(), t.Background(), and linkStyle).
| # Check if port is already in use before starting | ||
| with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: | ||
| if sock.connect_ex((host, port)) == 0: | ||
| raise Exception( | ||
| f"Port {port} is already in use. " | ||
| f"Please stop the other process or use a different port " | ||
| f"(set PORT environment variable).", | ||
| ) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IPv6 addresses not supported in port check.
The port check uses socket.AF_INET which only works for IPv4 addresses. If host is an IPv6 address (e.g., "::1" or "::"), the connect_ex call will fail with an error since IPv6 requires socket.AF_INET6.
Additionally, consider using a more specific exception type (e.g., RuntimeError or a custom exception) instead of the generic Exception.
🛠️ Proposed fix to handle both IPv4 and IPv6
# Check if port is already in use before starting
- with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock:
- if sock.connect_ex((host, port)) == 0:
- raise Exception(
+ try:
+ addr_info = socket.getaddrinfo(host, port, socket.AF_UNSPEC, socket.SOCK_STREAM)
+ family = addr_info[0][0]
+ except socket.gaierror:
+ family = socket.AF_INET # fallback to IPv4
+
+ with socket.socket(family, socket.SOCK_STREAM) as sock:
+ if sock.connect_ex((host, port)) == 0:
+ raise RuntimeError(
f"Port {port} is already in use. "
f"Please stop the other process or use a different port "
f"(set PORT environment variable).",
)🤖 Prompt for AI Agents
In `@rogue/run_server.py` around lines 140 - 148, The port-check currently binds
using socket.AF_INET which fails for IPv6 hosts; update the pre-start check
(where host and port are used and the socket.connect_ex call is made) to resolve
the host with socket.getaddrinfo and attempt a connect_ex for each returned
addrinfo using its provided family (AF_INET or AF_INET6) and sockaddr, raising a
more specific exception type like RuntimeError if any connect succeeds; ensure
the error message keeps the same guidance about stopping the other process or
changing PORT.
| # Check if port is already in use before starting | ||
| with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: | ||
| result = sock.connect_ex((host, port)) | ||
| if result == 0: | ||
| logger.error( | ||
| f"Port {port} is already in use. " | ||
| f"Please stop the other process or use a different port " | ||
| f"(set PORT environment variable).", | ||
| extra={"component": "server_main", "host": host, "port": port}, | ||
| ) | ||
| sys.exit(1) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, find and inspect the file
fd -t f "main.py" rogue/server/ 2>/dev/null | head -1Repository: qualifire-dev/rogue
Length of output: 83
🏁 Script executed:
# Read the specific section mentioned in the review (lines 94-105)
if [ -f "rogue/server/main.py" ]; then
echo "=== Lines 90-110 for context ==="
sed -n '90,110p' rogue/server/main.py | cat -n
else
echo "File not found at rogue/server/main.py"
# Try finding it
find . -name "main.py" -path "*/server/*" 2>/dev/null
fiRepository: qualifire-dev/rogue
Length of output: 918
🏁 Script executed:
# Check the imports at the top of the file to see if socket is imported
head -30 rogue/server/main.py | cat -nRepository: qualifire-dev/rogue
Length of output: 1078
🏁 Script executed:
# Find where host variable is set before line 94
sed -n '1,100p' rogue/server/main.py | grep -n "host\s*=" | tail -5Repository: qualifire-dev/rogue
Length of output: 45
🏁 Script executed:
# Get more context around the function to see host initialization
sed -n '70,95p' rogue/server/main.py | cat -nRepository: qualifire-dev/rogue
Length of output: 946
🏁 Script executed:
# Find where start_server is called
rg "start_server\(" rogue/server/main.pyRepository: qualifire-dev/rogue
Length of output: 117
🏁 Script executed:
# Get the full main block to see default host value
sed -n '100,150p' rogue/server/main.py | cat -nRepository: qualifire-dev/rogue
Length of output: 1243
🏁 Script executed:
# Verify socket behavior - check if socket.socket() can raise exceptions
python3 << 'EOF'
import socket
# Test normal case
try:
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
s.close()
print("Normal socket creation: OK, no exception")
except OSError as e:
print(f"Normal socket creation raised: {e}")
# Test connect_ex behavior (returns error code, doesn't raise)
try:
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
result = s.connect_ex(("127.0.0.1", 54321))
s.close()
print(f"connect_ex returned: {result} (does not raise exceptions)")
except OSError as e:
print(f"connect_ex raised: {e}")
# Test bind behavior (raises on failure)
try:
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
s.bind(("127.0.0.1", 1)) # try to bind to port 1 (needs root, will fail)
s.close()
except OSError as e:
print(f"bind raised OSError (expected): {type(e).__name__}")
EOFRepository: qualifire-dev/rogue
Length of output: 204
Use bind-based port check with exception handling per coding guidelines. The current connect_ex approach doesn't verify the port is actually available for binding—it only checks if something is listening. More importantly, a bind-based check is the standard pattern and requires exception handling per guidelines ("Use try/except blocks for code that may raise exceptions"). Replace the connect_ex check with a bind attempt wrapped in try/except:
🔧 Suggested fix
- # Check if port is already in use before starting
- with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock:
- result = sock.connect_ex((host, port))
- if result == 0:
- logger.error(
- f"Port {port} is already in use. "
- f"Please stop the other process or use a different port "
- f"(set PORT environment variable).",
- extra={"component": "server_main", "host": host, "port": port},
- )
- sys.exit(1)
+ # Check if port is already in use before starting
+ try:
+ with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock:
+ sock.bind((host, port))
+ except OSError as exc:
+ logger.error(
+ f"Port {port} is already in use or unavailable: {exc}. "
+ f"Please stop the other process or use a different port "
+ f"(set PORT environment variable).",
+ extra={"component": "server_main", "host": host, "port": port},
+ )
+ sys.exit(1)🤖 Prompt for AI Agents
In `@rogue/server/main.py` around lines 94 - 105, Replace the connect_ex-based
port check with a bind attempt inside a try/except: create the same
AF_INET/SOCK_STREAM socket (using the existing with socket.socket(...) as sock),
call sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) if desired, then
try sock.bind((host, port)); on success the port is available (socket will be
closed by the with block), on failure catch OSError/Exception, log the error
with logger.error including the exception details and the existing
extra={"component":"server_main","host":host,"port":port}, and call sys.exit(1).
Update the code that currently calls sock.connect_ex((host, port)) to this
bind/try/except pattern so the check verifies bindability per guidelines.
| "breakdown": [ | ||
| { | ||
| "id": vt.vulnerability_id, | ||
| "vulnerability_id": vt.vulnerability_id, | ||
| "name": vt.vulnerability_name, | ||
| "severity": vt.severity or "low", | ||
| "cvss_score": vt.success_rate, | ||
| "success_rate": vt.success_rate, | ||
| "description": ", ".join(vt.attacks_used), | ||
| "attacks": vt.attacks_used, | ||
| "attacks_attempted": vt.attacks_attempted, | ||
| "attacks_successful": vt.attacks_successful, | ||
| "status": vt.passed, | ||
| } | ||
| for vt in report.vulnerability_table | ||
| ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for VulnerabilityTable-related type definitions
rg -n "class.*Vulnerability.*Table" --type py -A 25
# Also search for the structure/attributes of vulnerability objects
rg -n "vulnerability_id|vulnerability_name|success_rate" --type py -B 3 -A 1 | head -100
# Check if there's a cvss_score attribute defined anywhere
rg -n "cvss_score" --type py -B 2 -A 2Repository: qualifire-dev/rogue
Length of output: 16798
🏁 Script executed:
#!/bin/bash
# Find the qualifire_service.py file and examine the context around the problematic lines
find . -name "qualifire_service.py" -type f
# Search for where vulnerability_table is defined/populated
rg -n "vulnerability_table" --type py -B 3 -A 3 | head -150Repository: qualifire-dev/rogue
Length of output: 8120
Remove or fix the cvss_score field mapping on line 133.
The VulnerabilityTableRow model does not have a cvss_score attribute. Assigning vt.success_rate to the "cvss_score" output field is incorrect because:
success_raterepresents attack success percentage (0-100)cvss_scoreshould represent a CVSS risk score (0-10)VulnerabilityTableRowlacks this data entirely
Either omit "cvss_score" from the output, or if it's required by the API contract, populate it from the underlying VulnerabilityResult objects that contain actual CVSS scores.
🤖 Prompt for AI Agents
In `@rogue/server/services/qualifire_service.py` around lines 127 - 142, The
current mapping in the list comprehension for report.vulnerability_table sets
"cvss_score": vt.success_rate which is incorrect because VulnerabilityTableRow
has no cvss_score; either remove the "cvss_score" key from the dict produced in
that comprehension (the one building "breakdown") or, if the API requires it,
pull the real CVSS value from the underlying VulnerabilityResult (e.g., access
the CVSS field on the associated vulnerability/result object) and assign that
instead (replace vt.success_rate with the correct attribute like
vulnerability_result.cvss_score or similar); update the list comprehension that
iterates report.vulnerability_table accordingly and ensure any callers expecting
cvss_score are adjusted.
| echo "" | ||
| printf " ${BOLD}${CYAN}Rogue Services${RESET} ${DIM}$(date +%H:%M:%S)${RESET}\n" | ||
| echo " ─────────────────────────────" | ||
| echo "" | ||
| check_service "Server" "http://localhost:8000/api/v1/health" "8000" | ||
| check_service "TShirt Agent" "http://localhost:10001/.well-known/agent.json" "10001" | ||
| check_service "Langgraph Agent" "http://localhost:10002/.well-known/agent.json" "10002" | ||
| echo "" | ||
| printf " ${GREEN}●${RESET} up ${RED}●${RESET} down ${DIM}refreshes every 5s${RESET}\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Status line claims auto-refresh but script runs once.
Line 31 says “refreshes every 5s” without any loop or sleep. Either add a refresh loop or adjust the text.
✏️ Minimal text fix
-printf " ${GREEN}●${RESET} up ${RED}●${RESET} down ${DIM}refreshes every 5s${RESET}\n"
+printf " ${GREEN}●${RESET} up ${RED}●${RESET} down ${DIM}run again to refresh${RESET}\n"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo "" | |
| printf " ${BOLD}${CYAN}Rogue Services${RESET} ${DIM}$(date +%H:%M:%S)${RESET}\n" | |
| echo " ─────────────────────────────" | |
| echo "" | |
| check_service "Server" "http://localhost:8000/api/v1/health" "8000" | |
| check_service "TShirt Agent" "http://localhost:10001/.well-known/agent.json" "10001" | |
| check_service "Langgraph Agent" "http://localhost:10002/.well-known/agent.json" "10002" | |
| echo "" | |
| printf " ${GREEN}●${RESET} up ${RED}●${RESET} down ${DIM}refreshes every 5s${RESET}\n" | |
| echo "" | |
| printf " ${BOLD}${CYAN}Rogue Services${RESET} ${DIM}$(date +%H:%M:%S)${RESET}\n" | |
| echo " ─────────────────────────────" | |
| echo "" | |
| check_service "Server" "http://localhost:8000/api/v1/health" "8000" | |
| check_service "TShirt Agent" "http://localhost:10001/.well-known/agent.json" "10001" | |
| check_service "Langgraph Agent" "http://localhost:10002/.well-known/agent.json" "10002" | |
| echo "" | |
| printf " ${GREEN}●${RESET} up ${RED}●${RESET} down ${DIM}run again to refresh${RESET}\n" |
🤖 Prompt for AI Agents
In `@scripts/healthcheck.sh` around lines 23 - 31, The status line claims the
script "refreshes every 5s" but there is no loop or sleep; wrap the display and
the three check_service calls (the block containing the printf with "Rogue
Services", the check_service calls for "Server", "TShirt Agent", "Langgraph
Agent", and the final printf showing refresh text) inside a while true loop that
clears the screen, runs those checks, then sleeps 5, or alternatively change the
final printf text to remove the "refreshes every 5s" claim; update
healthcheck.sh accordingly so the message matches behavior.
| description: "The core component that interacts with your agent." | ||
| --- | ||
|
|
||
| The `EvaluatorAgent` is the heart of Rogue. It's an autonomous AI agent designed to test your agent against a set of predefined scenarios. It operates using Google's A2A (Agent-to-Agent) communication protocol. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should include here the mcp and the python file support
| if msg.ReportURL != "" { | ||
| // URL format: https://app.qualifire.ai/red-team/{scan_id} | ||
| parts := strings.Split(msg.ReportURL, "/") | ||
| if len(parts) > 0 { | ||
| m.redTeamScanID = parts[len(parts)-1] | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of extracting the scan id from the url, can't we just pass the id instead of the url and build the URL?
I think it might be cleaner & less prune to bugs
Co-authored-by: yuval-qf <yuval@qualifire.ai>
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